[libc-commits] [PATCH] D143784: [libc] Add basic fuzz target for the printf parser

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/fuzzing/stdio/printf_parser_fuzz.cpp:40
+
+  auto parser = printf_core::Parser(in_str, mock_arg_list);
+
----------------
Other than passing it to the `Parser` constructor, I don't see `mock_arg_list` being used anywhere. What am I missing?


================
Comment at: libc/src/__support/arg_list.h:37
 
+#else  // not defined LIBC_COPT_MOCK_ARG_LIST
+
----------------
I am assuming that the use of this class is to get the number of args read out from the list. So, my suggestion is as follows. I would really not add a comment referencing printf or fuzz testing here. Also, you can add a mock class here unconditionally. The condition should be at the usage-site, which is the `Parser` class and the fuzzer, something like:

```
#ifdef LIBC_COPT_PRINTF_PARSER_WITH_MOCK_ARGS // Or a better name
using ArgProvider = MockArgList;
#else
using ArgProvider = ArgList;
#endif
```

Update `Parser` to take `ArgProvider`.


================
Comment at: libc/src/__support/arg_list.h:66
+    ++arg_counter;
+    return arg_counter;
+  }
----------------
Since it is a mock, you can just `return T();` and avoid implicit type casts and the `void*` specialization. Also, instead of overloading `next_var` to read the args fetched, you can add a new method, say:

```
  size_t read_count() const { return arg_counter; }
```




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143784/new/

https://reviews.llvm.org/D143784