Merge changes I342cd7d0,I2c55831b into qt-dev

* changes:
  Get incidentd cts working again.
  Don't include restricted images in incident reports unless they're specifically mentioned in the IncidentReportArgs

cmds/incident/main.cpp

@@ -263,6 +263,9 @@ main(int argc, char** argv)
                 return 1;
         }
     }
+    if (destination == DEST_UNSET) {
+        destination = DEST_STDOUT;
+    }
 
     string pkg;
     string cls;

cmds/incidentd/Android.bp

@@ -31,6 +31,7 @@ cc_binary {
         "-Wno-missing-field-initializers",
         "-Wno-unused-variable",
         "-Wunused-parameter",
+        "-Wno-tautological-undefined-compare",
 
         // Allow implicit fallthrough in IncidentService.cpp:85 until it is fixed.
         "-Wno-error=implicit-fallthrough",
@@ -96,6 +97,7 @@ cc_test {
         "-Wno-unused-variable",
         "-Wunused-parameter",
         "-g",
+        "-Wno-tautological-undefined-compare",
 
         // Allow implicit fallthrough in IncidentService.cpp:85 until it is fixed.
         "-Wno-error=implicit-fallthrough",

cmds/incidentd/src/IncidentService.cpp

@@ -174,12 +174,11 @@ void ReportHandler::schedule_send_broadcasts_locked() {
 }
 
 void ReportHandler::take_report() {
-    // Cycle the batch
+    // Cycle the batch and throttle.
     sp<ReportBatch> batch;
     {
         unique_lock<mutex> lock(mLock);
-        batch = mBatch;
-        mBatch = new ReportBatch();
+        batch = mThrottler->filterBatch(mBatch);
     }
 
     if (batch->empty()) {
@@ -189,13 +188,6 @@ void ReportHandler::take_report() {
 
     sp<Reporter> reporter = new Reporter(mWorkDirectory, batch);
 
-    // TODO: Do we really want to clear the reports if we throttle?  Should we only throttle
-    // requests going to dropbox?  How do we reconcile throttling with testing?
-    if (mThrottler->shouldThrottle()) {
-        ALOGW("RunReport got throttled.");
-        return;
-    }
-
     // Take the report, which might take a while. More requests might queue
     // up while we're doing this, and we'll handle them in their next batch.
     // TODO: We should further rate-limit the reports to no more than N per time-period.
@@ -203,7 +195,13 @@ void ReportHandler::take_report() {
     size_t reportByteSize = 0;
     reporter->runReport(&reportByteSize);
 
-    mThrottler->addReportSize(reportByteSize);
+    // Tell the throttler how big it was, for the next throttling.
+    // TODO: This still isn't ideal. The throttler really should just track the
+    // persisted reqeusts, but changing Reporter::runReport() to track that individually
+    // will be a big change.
+    if (batch->hasPersistedReports()) {
+        mThrottler->addReportSize(reportByteSize);
+    }
 
     // Kick off the next steps, one of which is to send any new or otherwise remaining
     // approvals, and one of which is to send any new or remaining broadcasts.
@@ -247,11 +245,11 @@ IncidentService::IncidentService(const sp<Looper>& handlerLooper) {
 IncidentService::~IncidentService() {}
 
 Status IncidentService::reportIncident(const IncidentReportArgs& args) {
-    // TODO: Validate that the privacy policy is one of the real ones.
-    // If it isn't, clamp it to the next more restrictive real one.
+    IncidentReportArgs argsCopy(args);
 
-    // TODO: This function should reject the LOCAL privacy policy.
-    // Those have to stream.
+    // Validate that the privacy policy is one of the real ones.
+    // If it isn't, clamp it to the next more restrictive real one.
+    argsCopy.setPrivacyPolicy(cleanup_privacy_policy(args.getPrivacyPolicy()));
 
     // TODO: Check that the broadcast recevier has the proper permissions
     // TODO: Maybe we should consider relaxing the permissions if it's going to
@@ -261,8 +259,15 @@ Status IncidentService::reportIncident(const IncidentReportArgs& args) {
         return status;
     }
 
+    // If they asked for the LOCAL privacy policy, give them EXPLICT.  LOCAL has to
+    // be streamed. (This only applies to shell/root, because everyone else would have
+    // been rejected by checkIncidentPermissions()).
+    if (argsCopy.getPrivacyPolicy() < PRIVACY_POLICY_EXPLICIT) {
+        ALOGI("Demoting privacy policy to EXPLICT for persisted report.");
+        argsCopy.setPrivacyPolicy(PRIVACY_POLICY_EXPLICIT);
+    }
+
     // If they didn't specify a component, use dropbox.
-    IncidentReportArgs argsCopy(args);
     if (argsCopy.receiverPkg().length() == 0 && argsCopy.receiverCls().length() == 0) {
         argsCopy.setReceiverPkg(DROPBOX_SENTINEL.getPackageName());
         argsCopy.setReceiverCls(DROPBOX_SENTINEL.getClassName());
@@ -276,22 +281,21 @@ Status IncidentService::reportIncident(const IncidentReportArgs& args) {
 Status IncidentService::reportIncidentToStream(const IncidentReportArgs& args,
                                                const sp<IIncidentReportStatusListener>& listener,
                                                const unique_fd& stream) {
-    // TODO: Validate that the privacy policy is one of the real ones.
-    // If it isn't, clamp it to the next more restrictive real one.
-
-    // TODO: Only shell should be able to do a LOCAL privacy policy report.
+    IncidentReportArgs argsCopy(args);
 
     // Streaming reports can not also be broadcast.
-    IncidentReportArgs argsCopy(args);
     argsCopy.setReceiverPkg("");
     argsCopy.setReceiverCls("");
 
+    // Validate that the privacy policy is one of the real ones.
+    // If it isn't, clamp it to the next more restrictive real one.
+    argsCopy.setPrivacyPolicy(cleanup_privacy_policy(args.getPrivacyPolicy()));
+
     Status status = checkIncidentPermissions(argsCopy);
     if (!status.isOk()) {
         return status;
     }
 
-
     // The ReportRequest takes ownership of the fd, so we need to dup it.
     int fd = dup(stream.get());
     if (fd < 0) {

cmds/incidentd/src/Privacy.cpp

@@ -18,17 +18,30 @@
 
 #include <android/os/IncidentReportArgs.h>
 #include <stdlib.h>
+#include <strstream>
+
 
 namespace android {
 namespace os {
 namespace incidentd {
 
 using namespace android::os;
+using std::strstream;
 
 static const bool kEncryptionEnabled = false;
 
 uint64_t encode_field_id(const Privacy* p) { return (uint64_t)p->type << 32 | p->field_id; }
 
+string Privacy::toString() const {
+    if (this == NULL) {
+        return "Privacy{null}";
+    }
+    strstream os;
+    os << "Privacy{field_id=" << field_id << " type=" << ((int)type)
+            << " children=" << ((void*)children) << " policy=" << ((int)policy) << "}";
+    return os.str();
+}
+
 const Privacy* lookup(const Privacy* p, uint32_t fieldId) {
     if (p->children == NULL) return NULL;
     for (int i = 0; p->children[i] != NULL; i++) {  // NULL-terminated.
@@ -87,6 +100,16 @@ bool PrivacySpec::RequireAll() const {
     return mPolicy == android::os::PRIVACY_POLICY_LOCAL;
 }
 
+uint8_t cleanup_privacy_policy(uint8_t policy) {
+    if (policy >= PRIVACY_POLICY_AUTOMATIC) {
+        return PRIVACY_POLICY_AUTOMATIC;
+    }
+    if (policy >= PRIVACY_POLICY_EXPLICIT) {
+        return PRIVACY_POLICY_EXPLICIT;
+    }
+    return PRIVACY_POLICY_LOCAL;
+}
+
 }  // namespace incidentd
 }  // namespace os
 }  // namespace android

cmds/incidentd/src/Privacy.h

@@ -50,8 +50,11 @@ struct Privacy {
 
     // DESTINATION Enum in frameworks/base/core/proto/android/privacy.proto.
     uint8_t policy;
+
     // A list of regexp rules for stripping string fields in proto.
     const char** patterns;
+
+    string toString() const;
 };
 
 // Encode field id used by ProtoOutputStream.
@@ -90,6 +93,11 @@ private:
 // TODO: Add privacy flag in incident.proto and auto generate it inside Privacy.
 bool sectionEncryption(int section_id);
 
+/**
+ * If a privacy policy is other than the defined values, update it to a real one.
+ */
+uint8_t cleanup_privacy_policy(uint8_t policy);
+
 }  // namespace incidentd
 }  // namespace os
 }  // namespace android

cmds/incidentd/src/PrivacyFilter.cpp

@@ -18,6 +18,10 @@
 
 #include "PrivacyFilter.h"
 
+#include "incidentd_util.h"
+#include "proto_util.h"
+#include "Section.h"
+
 #include <android-base/file.h>
 #include <android/util/ProtoFileReader.h>
 #include <android/util/protobuf.h>
@@ -25,8 +29,6 @@
 
 #include "cipher/IncidentKeyStore.h"
 #include "cipher/ProtoEncryption.h"
-#include "incidentd_util.h"
-#include "proto_util.h"
 
 namespace android {
 namespace os {
@@ -190,7 +192,7 @@ status_t FieldStripper::strip(const uint8_t privacyPolicy) {
         ProtoOutputStream proto;
 
         // Optimization when no strip happens.
-        if (mRestrictions == NULL || mRestrictions->children == NULL || spec.RequireAll()) {
+        if (mRestrictions == NULL || spec.RequireAll()) {
             if (spec.CheckPremission(mRestrictions)) {
                 mSize = mData->size();
             }
@@ -220,6 +222,11 @@ status_t FieldStripper::strip(const uint8_t privacyPolicy) {
 status_t FieldStripper::writeData(int fd) {
     status_t err = NO_ERROR;
     sp<ProtoReader> reader = mData;
+    if (mData == nullptr) {
+        // There had been an error processing the data. We won't write anything,
+        // but we also won't return an error, because errors are fatal.
+        return NO_ERROR;
+    }
     while (reader->readBuffer() != NULL) {
         err = WriteFully(fd, reader->readBuffer(), reader->currentToRead()) ? NO_ERROR : -errno;
         reader->move(reader->currentToRead());
@@ -316,7 +323,7 @@ status_t PrivacyFilter::writeData(const FdBuffer& buffer, uint8_t bufferLevel, s
             if (err != NO_ERROR) {
                 // We can't successfully strip this data.  We will skip
                 // the rest of this section.
-                return err;
+                return NO_ERROR;
             }
         }
 
@@ -367,8 +374,8 @@ status_t filter_and_write_report(int to, int from, uint8_t bufferLevel,
         uint64_t fieldTag = reader->readRawVarint();
         uint32_t fieldId = read_field_id(fieldTag);
         uint8_t wireType = read_wire_type(fieldTag);
-        if (wireType == WIRE_TYPE_LENGTH_DELIMITED && args.containsSection(fieldId)) {
-            VLOG("Read section %d", fieldId);
+        if (wireType == WIRE_TYPE_LENGTH_DELIMITED
+                && args.containsSection(fieldId, section_requires_specific_mention(fieldId))) {
             // We need this field, but we need to strip it to the level provided in args.
             PrivacyFilter filter(fieldId, get_privacy_of_section(fieldId));
             filter.addFd(new ReadbackFilterFd(args.getPrivacyPolicy(), to));

cmds/incidentd/src/Reporter.cpp

@@ -66,22 +66,18 @@ IncidentMetadata_Destination privacy_policy_to_dest(uint8_t privacyPolicy) {
     }
 }
 
-void poo_make_metadata(IncidentMetadata* result, const IncidentMetadata& full,
-        int64_t reportId, int32_t privacyPolicy, const IncidentReportArgs& args) {
-    result->set_report_id(reportId);
-    result->set_dest(privacy_policy_to_dest(privacyPolicy));
 
-    size_t sectionCount = full.sections_size();
-    for (int sectionIndex = 0; sectionIndex < sectionCount; sectionIndex++) {
-        const IncidentMetadata::SectionStats& sectionStats = full.sections(sectionIndex);
-        if (args.containsSection(sectionStats.id())) {
-            *result->add_sections() = sectionStats;
-        }
-    }
+static bool contains_section(const IncidentReportArgs& args, int sectionId) {
+    return args.containsSection(sectionId, section_requires_specific_mention(sectionId));
+}
+
+static bool contains_section(const sp<ReportRequest>& args, int sectionId) {
+    return args->containsSection(sectionId);
 }
 
 // ARGS must have a containsSection(int) method
-template <typename ARGS> void make_metadata(IncidentMetadata* result, const IncidentMetadata& full,
+template <typename ARGS>
+void make_metadata(IncidentMetadata* result, const IncidentMetadata& full,
         int64_t reportId, int32_t privacyPolicy, ARGS args) {
     result->set_report_id(reportId);
     result->set_dest(privacy_policy_to_dest(privacyPolicy));
@@ -89,7 +85,7 @@ template <typename ARGS> void make_metadata(IncidentMetadata* result, const Inci
     size_t sectionCount = full.sections_size();
     for (int sectionIndex = 0; sectionIndex < sectionCount; sectionIndex++) {
         const IncidentMetadata::SectionStats& sectionStats = full.sections(sectionIndex);
-        if (args->containsSection(sectionStats.id())) {
+        if (contains_section(args, sectionStats.id())) {
             *result->add_sections() = sectionStats;
         }
     }
@@ -160,6 +156,10 @@ bool ReportRequest::ok() {
     return mFd >= 0 && mStatus == NO_ERROR;
 }
 
+bool ReportRequest::containsSection(int sectionId) const {
+    return args.containsSection(sectionId, section_requires_specific_mention(sectionId));
+}
+
 void ReportRequest::closeFd() {
     if (mIsStreaming && mFd >= 0) {
         close(mFd);
@@ -286,6 +286,22 @@ void ReportBatch::clearPersistedRequests() {
     mPersistedRequests.clear();
 }
 
+void ReportBatch::transferStreamingRequests(const sp<ReportBatch>& that) {
+    for (vector<sp<ReportRequest>>::iterator request = mStreamingRequests.begin();
+            request != mStreamingRequests.end(); request++) {
+        that->mStreamingRequests.push_back(*request);
+    }
+    mStreamingRequests.clear();
+}
+
+void ReportBatch::transferPersistedRequests(const sp<ReportBatch>& that) {
+    for (map<ComponentName, sp<ReportRequest>>::iterator it = mPersistedRequests.begin();
+            it != mPersistedRequests.end(); it++) {
+        that->mPersistedRequests[it->first] = it->second;
+    }
+    mPersistedRequests.clear();
+}
+
 void ReportBatch::getFailedRequests(vector<sp<ReportRequest>>* requests) {
     for (map<ComponentName, sp<ReportRequest>>::iterator it = mPersistedRequests.begin();
             it != mPersistedRequests.end(); it++) {
@@ -441,7 +457,9 @@ status_t ReportWriter::writeSection(const FdBuffer& buffer) {
 
     // Add the fds for the streamed requests
     mBatch->forEachStreamingRequest([&filter, this](const sp<ReportRequest>& request) {
-        if (request->ok() && request->args.containsSection(mCurrentSectionId)) {
+        if (request->ok()
+                && request->args.containsSection(mCurrentSectionId,
+                    section_requires_specific_mention(mCurrentSectionId))) {
             filter.addFd(new StreamingFilterFd(request->args.getPrivacyPolicy(),
                         request->getFd(), request));
         }
@@ -619,7 +637,7 @@ DONE:
         mBatch->getCombinedPersistedArgs(&combinedArgs);
         IncidentMetadata persistedMetadata;
         make_metadata(&persistedMetadata, metadata, mPersistedFile->getTimestampNs(),
-                persistedPrivacyPolicy, &combinedArgs);
+                persistedPrivacyPolicy, combinedArgs);
         mPersistedFile->setMetadata(persistedMetadata);
 
         mPersistedFile->markCompleted();

cmds/incidentd/src/Reporter.h

@@ -16,7 +16,6 @@
 #pragma once
 
 #include "FdBuffer.h"
-#include "Throttler.h"
 #include "WorkDirectory.h"
 
 #include "frameworks/base/core/proto/android/os/metadata.pb.h"
@@ -58,7 +57,7 @@ public:
 
     bool ok();  // returns true if the request is ok for write.
 
-    bool containsSection(int sectionId) const { return args.containsSection(sectionId); }
+    bool containsSection(int sectionId) const;
 
     sp<IIncidentReportStatusListener> getListener() { return mListener; }
 
@@ -154,6 +153,18 @@ public:
      */
     void clearPersistedRequests();
 
+    /**
+     * Move the streaming requests in this batch to that batch.  After this call there
+     * will be no streaming requests in this batch.
+     */
+    void transferStreamingRequests(const sp<ReportBatch>& that);
+
+    /**
+     * Move the persisted requests in this batch to that batch.  After this call there
+     * will be no streaming requests in this batch.
+     */
+    void transferPersistedRequests(const sp<ReportBatch>& that);
+
     /**
      * Get the requests that have encountered errors.
      */

cmds/incidentd/src/Section.cpp

@@ -63,6 +63,15 @@ static pid_t fork_execute_incident_helper(const int id, Fpipe* p2cPipe, Fpipe* c
     return fork_execute_cmd(const_cast<char**>(ihArgs), p2cPipe, c2pPipe);
 }
 
+bool section_requires_specific_mention(int sectionId) {
+    switch (sectionId) {
+        case 3025: // restricted_images
+            return true;
+        default:
+            return false;
+    }
+}
+
 // ================================================================================
 Section::Section(int i, int64_t timeoutMs)
     : id(i),

cmds/incidentd/src/Section.h

@@ -171,6 +171,13 @@ private:
     std::string mType;
 };
 
+
+/**
+ * These sections will not be generated when doing an 'all' report, either
+ * for size, speed of collection, or privacy.
+ */
+bool section_requires_specific_mention(int sectionId);
+
 }  // namespace incidentd
 }  // namespace os
 }  // namespace android

cmds/incidentd/src/Throttler.cpp

@@ -33,6 +33,21 @@ Throttler::Throttler(size_t limit, int64_t refractoryPeriodMs)
 
 Throttler::~Throttler() {}
 
+sp<ReportBatch> Throttler::filterBatch(const sp<ReportBatch>& queued) {
+    sp<ReportBatch> result = new ReportBatch();
+
+    // We will never throttle the streaming ones.
+    queued->transferStreamingRequests(result);
+
+    // If the persisted ones aren't to be throttled, then add them to the
+    // batch we're going to do.
+    if (!shouldThrottle()) {
+        queued->transferPersistedRequests(result);
+    }
+
+    return result;
+}
+
 bool Throttler::shouldThrottle() {
     int64_t now = android::elapsedRealtime();
     if (now > mRefractoryPeriodMs + mLastRefractoryMs) {
@@ -56,4 +71,4 @@ void Throttler::dump(FILE* out) {
 
 }  // namespace incidentd
 }  // namespace os
-}  // namespace android
+}  // namespace android

cmds/incidentd/src/Throttler.h

@@ -14,16 +14,19 @@
  * limitations under the License.
  */
 
-#ifndef THROTTLER_H
-#define THROTTLER_H
+#pragma once
+
+#include "Reporter.h"
 
 #include <utils/RefBase.h>
 
+#include <vector>
 #include <unistd.h>
 
 namespace android {
 namespace os {
 namespace incidentd {
+
 /**
  * This is a size-based throttler which prevents incidentd to take more data.
  */
@@ -33,8 +36,11 @@ public:
     ~Throttler();
 
     /**
-     * Asserts this before starting taking report.
+     * Return a batch containing reports, if any that should be executed.
+     * Those will be removed from 'queued'.
      */
+    sp<ReportBatch> filterBatch(const sp<ReportBatch>& queued);
+
     bool shouldThrottle();
 
     void addReportSize(size_t reportByteSize);
@@ -52,5 +58,3 @@ private:
 }  // namespace incidentd
 }  // namespace os
 }  // namespace android
-
-#endif  // THROTTLER_H

cmds/incidentd/tests/FdBuffer_test.cpp

@@ -233,8 +233,8 @@ TEST_F(FdBufferTest, ReadInStreamEmpty) {
 }
 
 TEST_F(FdBufferTest, ReadInStreamMoreThan4MBWithMove) {
-    const std::string testFile = kTestDataPath + "morethan4MB.txt";
-    size_t fourMB = (size_t)4 * 1024 * 1024;
+    const std::string testFile = kTestDataPath + "morethan24MB.txt";
+    size_t twentyfourMB = (size_t)24 * 1024 * 1024;
     unique_fd fd(open(testFile.c_str(), O_RDONLY | O_CLOEXEC));
     ASSERT_NE(fd.get(), -1);
     int pid = fork();
@@ -254,21 +254,21 @@ TEST_F(FdBufferTest, ReadInStreamMoreThan4MBWithMove) {
         ASSERT_EQ(NO_ERROR,
                   buffer.readProcessedDataInStream(fd, std::move(p2cPipe.writeFd()),
                                                    std::move(c2pPipe.readFd()), READ_TIMEOUT));
-        EXPECT_EQ(buffer.size(), fourMB);
+        EXPECT_EQ(buffer.size(), twentyfourMB);
         EXPECT_FALSE(buffer.timedOut());
         EXPECT_TRUE(buffer.truncated());
         wait(&pid);
         sp<ProtoReader> reader = buffer.data()->read();
-        reader->move(fourMB);
+        reader->move(twentyfourMB);
 
-        EXPECT_EQ(reader->bytesRead(), fourMB);
+        EXPECT_EQ(reader->bytesRead(), twentyfourMB);
         EXPECT_FALSE(reader->hasNext());
     }
 }
 
 TEST_F(FdBufferTest, ReadInStreamMoreThan4MBWithNext) {
-    const std::string testFile = kTestDataPath + "morethan4MB.txt";
-    size_t fourMB = (size_t)4 * 1024 * 1024;
+    const std::string testFile = kTestDataPath + "morethan24MB.txt";
+    size_t twentyfourMB = (size_t)24 * 1024 * 1024;
     unique_fd fd(open(testFile.c_str(), O_RDONLY | O_CLOEXEC));
     ASSERT_NE(fd.get(), -1);
     int pid = fork();
@@ -288,7 +288,7 @@ TEST_F(FdBufferTest, ReadInStreamMoreThan4MBWithNext) {
         ASSERT_EQ(NO_ERROR,
                   buffer.readProcessedDataInStream(fd, std::move(p2cPipe.writeFd()),
                                                    std::move(c2pPipe.readFd()), READ_TIMEOUT));
-        EXPECT_EQ(buffer.size(), fourMB);
+        EXPECT_EQ(buffer.size(), twentyfourMB);
         EXPECT_FALSE(buffer.timedOut());
         EXPECT_TRUE(buffer.truncated());
         wait(&pid);

cmds/incidentd/tests/Reporter_test.cpp

@@ -144,9 +144,9 @@ TEST_F(ReporterTest, IncidentReportArgs) {
     args2.addSection(3);
 
     args1.merge(args2);
-    ASSERT_TRUE(args1.containsSection(1));
-    ASSERT_FALSE(args1.containsSection(2));
-    ASSERT_TRUE(args1.containsSection(3));
+    ASSERT_TRUE(args1.containsSection(1, false));
+    ASSERT_FALSE(args1.containsSection(2, false));
+    ASSERT_TRUE(args1.containsSection(3, false));
 }
 
 /*

libs/incident/include/android/os/IncidentReportArgs.h

@@ -53,7 +53,7 @@ public:
     void addHeader(const vector<uint8_t>& headerProto);
 
     inline bool all() const { return mAll; }
-    bool containsSection(int section) const;
+    bool containsSection(int section, bool specific) const;
     inline int getPrivacyPolicy() const { return mPrivacyPolicy; }
     inline const set<int>& sections() const { return mSections; }
     inline const string& receiverPkg() const { return mReceiverPkg; }

libs/incident/src/IncidentReportArgs.cpp

@@ -194,9 +194,13 @@ IncidentReportArgs::addHeader(const vector<uint8_t>& headerProto)
 }
 
 bool
-IncidentReportArgs::containsSection(int section) const
+IncidentReportArgs::containsSection(int section, bool specific) const
 {
-     return mAll || mSections.find(section) != mSections.end();
+    if (specific) {
+        return mSections.find(section) != mSections.end();
+    } else {
+        return mAll || mSections.find(section) != mSections.end();
+    }
 }
 
 void

tools/incident_section_gen/main.cpp

@@ -469,6 +469,7 @@ static bool generateSectionListCpp(Descriptor const* descriptor) {
         const FieldDescriptor* field = fieldsInOrder[i];
         const string fieldName = getFieldName(field);
         const Destination fieldDest = getFieldDest(field);
+        printf("\n// Incident Report Section: %s (%d)\n", field->name().c_str(), field->number());
         if (field->type() != FieldDescriptor::TYPE_MESSAGE) {
             printPrivacy(fieldName, field, "NULL", fieldDest, "NULL");
             continue;
@@ -477,9 +478,11 @@ static bool generateSectionListCpp(Descriptor const* descriptor) {
         skip[i] = true;
         const string fieldMessageName = getMessageName(field->message_type(), fieldDest);
         // generate privacy flags for each section.
-        if (generatePrivacyFlags(field->message_type(), fieldDest, variableNames, &parents)) {
+        if (generatePrivacyFlags(field->message_type(), incidentDest, variableNames, &parents)) {
             printPrivacy(fieldName, field, fieldMessageName, fieldDest, "NULL");
-        } else if (isDefaultField(field, incidentDest)) {
+        } else if (fieldDest == incidentDest) {
+            printf("// default %s: fieldDest=%d incidentDest=%d\n", fieldName.c_str(),
+                    getFieldDest(field), incidentDest);
             continue; // don't create a new privacy if the value is default.
         } else {
             printPrivacy(fieldName, field, "NULL", fieldDest, "NULL");