[libc-commits] [PATCH] D84611: [libc] Adds fuzz test for strstr and alphabetizes string fuzz CMakeList.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

Few nits and questions inline but overall LGTM.



================
Comment at: libc/fuzzing/string/strstr_fuzz.cpp:21
+// Simple loop to compare two strings up to a size n.
+int raw_strcmp(const char *left, const char *right, size_t n) {
+  for (; n && *left == *right; ++left, ++right, --n)
----------------
LLVM style is to use `static` functions over anonymous namespaces: https://llvm.org/docs/CodingStandards.html#anonymous-namespaces


================
Comment at: libc/fuzzing/string/strstr_fuzz.cpp:21
+// Simple loop to compare two strings up to a size n.
+int raw_strcmp(const char *left, const char *right, size_t n) {
+  for (; n && *left == *right; ++left, ++right, --n)
----------------
sivachandra wrote:
> LLVM style is to use `static` functions over anonymous namespaces: https://llvm.org/docs/CodingStandards.html#anonymous-namespaces
May be call it `simple_memcmp`.


================
Comment at: libc/fuzzing/string/strstr_fuzz.cpp:64
+
+  for (size_t j = 0; j < size2; ++j)
+    data2[j] = data[i++];
----------------
`data1` should be copied so that we can add the null terminator. But, why should we copy into `data2`? Can we not use the input directly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84611