Add fuzzer test for preg_parser

components/policy/core/common/preg_parser.h

Index: components/policy/core/common/preg_parser.h
diff --git a/components/policy/core/common/preg_parser.h b/components/policy/core/common/preg_parser.h
index 03872a47a182bdd77a15b3f303789c0dc666fe2f..573b6b7453d4eb14aa928e79a6c5b36ab9a2df04 100644
--- a/components/policy/core/common/preg_parser.h
+++ b/components/policy/core/common/preg_parser.h
@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "base/strings/string16.h"
+#include "components/policy/core/common/policy_load_status.h"

[Thiemo Nagel, 2017/04/12 17:10:06]

Why have you removed the forward declaration of PolicyLoadStatusSample? -- I'd
have thought that should cover you.

[ljusten (tachyonic), 2017/04/13 11:17:15]

The internal method takes a PolicyLoadStatus, which is an enum in this .h file.
PolicyLoadStatusSample sends off a UMA sample at destruction, which fills up
memory in the fuzzer.

 #include "components/policy/policy_export.h"
 
 namespace base {
@@ -22,7 +23,6 @@ class FilePath;
 
 namespace policy {
 
-class PolicyLoadStatusSample;
 class RegistryDict;
 
 namespace preg_parser {
@@ -31,12 +31,24 @@ namespace preg_parser {
 POLICY_EXPORT extern const char kPRegFileHeader[8];
 
 // Reads the PReg file at |file_path| and writes the registry data to |dict|.
-// |root| specifies the registry subtree the caller is interested in,
-// everything else gets ignored.
+// |root| specifies the registry subtree the caller is interested in, everything
+// else gets ignored.
 POLICY_EXPORT bool ReadFile(const base::FilePath& file_path,
                             const base::string16& root,
                             RegistryDict* dict,
-                            PolicyLoadStatusSample* status);
+                            PolicyLoadStatusSample* status_sample);
+
+// Similar to ReadFile, but reads from |data| of length |data_size| instead of
+// a file, and writes status to the enum PolicyLoadStatus, which does not record
+// UMA stats, unlike |PolicyLoadStatusSample|. |debug_name| is printed out along
+// with error messages. Used internally only for testing. For production code,

[Thiemo Nagel, 2017/04/12 17:10:06]

Nit: "only for testing" seems not correct as it's used for the implementation of
ReadFile() as well.

[ljusten (tachyonic), 2017/04/13 11:17:15]

Clarified the wording.

+// use ReadFile instead.
+POLICY_EXPORT bool ReadDataInternal(const uint8_t* data,
+                                    size_t data_size,
+                                    const base::string16& root,
+                                    RegistryDict* dict,
+                                    PolicyLoadStatus* status,

[Thiemo Nagel, 2017/04/12 17:10:06]

Nit: Returning a bool *and* updating |status| seems redundant.

[ljusten (tachyonic), 2017/04/13 11:17:14]

PolicyLoadStatus is used as UMA stat and does not have an 'OK' status. Adding an
OK status at the end of the list or using POLICY_LOAD_STATUS_SIZE to signal 'OK'
would feel a bit hacky.

+                                    const std::string& debug_name);
 
 }  // namespace preg_parser
 }  // namespace policy