Fixed several bugs found for incidentd

1. Add logging for proto can't be parsed by incident_report
2. Close opened file descriptor of incident report request!
3. Fix invalid syntax of auto-gen section_list.cpp

Bug: 67849582
Test: manually run incident and checks /proc/<its pid>/fd
Change-Id: I38e6ac28de09aca6243ad536ed41d8059e08ba24

6 files changed, 31 insertions, 16 deletions

diff --git a/cmds/incidentd/src/PrivacyBuffer.cpp b/cmds/incidentd/src/PrivacyBuffer.cpp
index a095afc194da..d926ea7a9c87 100644
--- a/cmds/incidentd/src/PrivacyBuffer.cpp
+++ b/cmds/incidentd/src/PrivacyBuffer.cpp
@@ -20,7 +20,6 @@
 #include "io_util.h"
 
 #include <android/util/protobuf.h>
-#include <deque>
 
 using namespace android::util;
 
diff --git a/cmds/incidentd/src/Reporter.cpp b/cmds/incidentd/src/Reporter.cpp
index 917b70d00332..34930aa57321 100644
--- a/cmds/incidentd/src/Reporter.cpp
+++ b/cmds/incidentd/src/Reporter.cpp
@@ -48,6 +48,10 @@ ReportRequest::ReportRequest(const IncidentReportArgs& a,
 
 ReportRequest::~ReportRequest()
 {
+    if (fd >= 0) {
+        // clean up the opened file descriptor
+        close(fd);
+    }
 }
 
 bool
diff --git a/cmds/incidentd/src/report_directory.cpp b/cmds/incidentd/src/report_directory.cpp
index 110902c9b9ad..65030b3a1799 100644
--- a/cmds/incidentd/src/report_directory.cpp
+++ b/cmds/incidentd/src/report_directory.cpp
@@ -97,7 +97,8 @@ create_directory(const char* directory)
         err = BAD_VALUE;
         goto done;
     }
-    if (st.st_uid != AID_SYSTEM || st.st_gid != AID_SYSTEM) {
+    if ((st.st_uid != AID_SYSTEM && st.st_uid != AID_ROOT) ||
+        (st.st_gid != AID_SYSTEM && st.st_gid != AID_ROOT)) {
         ALOGE("No incident reports today. Owner is %d and group is %d on report directory %s",
                 st.st_uid, st.st_gid, directory);
         err = BAD_VALUE;
diff --git a/core/proto/android/os/pagetypeinfo.proto b/core/proto/android/os/pagetypeinfo.proto
index 38f1890a8338..f82ea7672879 100644
--- a/core/proto/android/os/pagetypeinfo.proto
+++ b/core/proto/android/os/pagetypeinfo.proto
@@ -13,8 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
 syntax = "proto2";
+
 option java_multiple_files = true;
 option java_outer_classname = "PageTypeInfoProto";
 
diff --git a/tools/incident_report/main.cpp b/tools/incident_report/main.cpp
index cc252649da96..bd1b973c7bdf 100644
--- a/tools/incident_report/main.cpp
+++ b/tools/incident_report/main.cpp
@@ -45,8 +45,9 @@ static bool
 read_length_delimited(CodedInputStream* in, uint32 fieldId, Descriptor const* descriptor,
         GenericMessage* message)
 {
-    uint32 size;
+    uint32_t size;
     if (!in->ReadVarint32(&size)) {
+        fprintf(stderr, "Fail to read size of %s\n", descriptor->name().c_str());
         return false;
     }
 
@@ -68,6 +69,9 @@ read_length_delimited(CodedInputStream* in, uint32 fieldId, Descriptor const* de
                 message->addString(fieldId, str);
                 return true;
             } else {
+                fprintf(stderr, "Fail to read string of field %s, expect size %d, read %lu\n",
+                        field->full_name().c_str(), size, str.size());
+                fprintf(stderr, "String read \"%s\"\n", str.c_str());
                 return false;
             }
         } else if (type == FieldDescriptor::TYPE_BYTES) {
diff --git a/tools/incident_section_gen/main.cpp b/tools/incident_section_gen/main.cpp
index 900690cf36d6..135df405c747 100644
--- a/tools/incident_section_gen/main.cpp
+++ b/tools/incident_section_gen/main.cpp
@@ -113,7 +113,7 @@ static inline bool isDefaultDest(const FieldDescriptor* field) {
     return field->options().GetExtension(privacy).dest() == PrivacyFlags::default_instance().dest();
 }
 
-// Returns true if the descriptor doesn't have any non default privacy flags set, including its submessages
+// Returns false if the descriptor doesn't have any non default privacy flags set, including its submessages
 static bool generatePrivacyFlags(const Descriptor* descriptor, const char* alias, map<string, bool> &msgNames) {
     bool hasDefaultFlags[descriptor->field_count()];
     // iterate though its field and generate sub flags first
@@ -129,13 +129,18 @@ static bool generatePrivacyFlags(const Descriptor* descriptor, const char* alias
         };
 
         PrivacyFlags p = field->options().GetExtension(privacy);
-
         switch (field->type()) {
             case FieldDescriptor::TYPE_MESSAGE:
-                if (generatePrivacyFlags(field->message_type(), field_name, msgNames) &&
-                    isDefaultDest(field)) break;
-
-                printf("Privacy %s { %d, %d, %s_LIST, %d, NULL };\n", field_name, field->number(), field->type(), field_name, p.dest());
+                if (generatePrivacyFlags(field->message_type(), field_name, msgNames)) {
+                    printf("Privacy %s { %d, %d, %s_LIST, %d, NULL };\n", field_name, field->number(),
+                            field->type(), field_name, p.dest());
+                } else if (isDefaultDest(field)) {
+                    // don't create a new privacy if the value is default.
+                    break;
+                } else{
+                    printf("Privacy %s { %d, %d, NULL, %d, NULL };\n", field_name, field->number(),
+                            field->type(), p.dest());
+                }
                 hasDefaultFlags[i] = false;
                 break;
             case FieldDescriptor::TYPE_STRING:
@@ -147,12 +152,14 @@ static bool generatePrivacyFlags(const Descriptor* descriptor, const char* alias
                     printf("    \"%s\",\n", replaceAll(p.patterns(i), '\\', "\\\\").c_str());
                 }
                 printf("    NULL };\n");
-                printf("Privacy %s { %d, %d, NULL, %d, %s_patterns };\n", field_name, field->number(), field->type(), p.dest(), field_name);
+                printf("Privacy %s { %d, %d, NULL, %d, %s_patterns };\n", field_name, field->number(),
+                        field->type(), p.dest(), field_name);
                 hasDefaultFlags[i] = false;
                 break;
             default:
                 if (isDefaultDest(field)) break;
-                printf("Privacy %s { %d, %d, NULL, %d, NULL };\n", field_name, field->number(), field->type(), p.dest());
+                printf("Privacy %s { %d, %d, NULL, %d, NULL };\n", field_name, field->number(),
+                        field->type(), p.dest());
                 hasDefaultFlags[i] = false;
         }
         // add the field name to message map, true means it has default flags
@@ -163,14 +170,14 @@ static bool generatePrivacyFlags(const Descriptor* descriptor, const char* alias
     for (int i=0; i<descriptor->field_count(); i++) {
         allDefaults &= hasDefaultFlags[i];
     }
-    if (allDefaults) return true;
+    if (allDefaults) return false;
 
     emptyline();
 
     bool needConst = strcmp(alias, "PRIVACY_POLICY") == 0;
     int policyCount = 0;
 
-    printf("%s Privacy* %s_LIST[] = {\n", needConst ? "const" : "", alias);
+    printf("%sPrivacy* %s_LIST[] = {\n", needConst ? "const " : "", alias);
     for (int i=0; i<descriptor->field_count(); i++) {
         const FieldDescriptor* field = descriptor->field(i);
         if (hasDefaultFlags[i]) continue;
@@ -184,7 +191,7 @@ static bool generatePrivacyFlags(const Descriptor* descriptor, const char* alias
         printf("    NULL };\n");
     }
     emptyline();
-    return false;
+    return true;
 }
 
 static bool generateSectionListCpp(Descriptor const* descriptor) {
@@ -222,7 +229,7 @@ static bool generateSectionListCpp(Descriptor const* descriptor) {
 
     // generates PRIVACY_POLICY
     map<string, bool> messageNames;
-    if (generatePrivacyFlags(descriptor, "PRIVACY_POLICY", messageNames)) {
+    if (!generatePrivacyFlags(descriptor, "PRIVACY_POLICY", messageNames)) {
         // if no privacy options set at all, define an empty list
         printf("const Privacy* PRIVACY_POLICY_LIST[] = {};\n");
         printf("const int PRIVACY_POLICY_COUNT = 0;\n");