[PATCH] D36324: Integrate Kostya's clang-proto-fuzzer with LLVM.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.


================
Comment at: clang/tools/clang-fuzzer/cxx_proto.proto:17
+syntax = "proto2";
+//option cc_api_version = 2;
+
----------------
>> //option cc_api_version = 2;
Please remove


================
Comment at: clang/tools/clang-fuzzer/proto-to-cxx/CMakeLists.txt:7
+
+add_clang_library(clangProtoToCXX
+  proto_to_cxx.cpp
----------------
Formatting of this statement looks weird


================
Comment at: clang/tools/clang-fuzzer/proto-to-cxx/proto_to_cxx.cpp:46
+  switch (x.op()) {
+#define OP(a, b) case BinaryOp::a: os << b; break
+    OP(PLUS, "+");
----------------
 OP looks to trivial to potentially get into conflicts with some 3rd party macro


================
Comment at: clang/tools/clang-fuzzer/proto-to-cxx/proto_to_cxx.cpp:50
+    OP(MUL, "*");
+    OP(DIV, "/");
+    OP(MOD, "%");
----------------
```
switch (x.op()) {
    BinaryOp::PLUS: os << "+"; break
    BinaryOp::MINUS: os << "-"; break
    BinaryOp::MUL: os << "*"; break
```

does not look bad to me


================
Comment at: clang/tools/clang-fuzzer/proto-to-cxx/proto_to_cxx.cpp:102
+  if (!message.ParseFromArray(data, size))
+//   if (!proto2::TextFormat::ParseBinaryMessage({data, data + size}, 
&message))
+    return "#error invalid proto\n";
----------------
please remove commented code


https://reviews.llvm.org/D36324



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits