[PATCH] [lib/Fuzzer] Add SHA1 implementation from public domain.

Kostya Serebryany kcc at google.com
Thu May 14 10:28:27 PDT 2015


done all, PTAL


================
Comment at: lib/Fuzzer/CMakeLists.txt:13
@@ -12,2 +12,3 @@
     FuzzerSanitizerOptions.cpp
+    FuzzerSha1.cpp
     FuzzerUtil.cpp
----------------
chandlerc wrote:
> This should be spelled FuzzerSHA1.cpp
done

================
Comment at: lib/Fuzzer/FuzzerInternal.h:47-49
@@ -46,1 +46,5 @@
 
+// sha1
+static const int kSha1NumBytes = 20;
+void ComputeSha1(const uint8_t *Data, size_t Len, uint8_t Out[kSha1NumBytes]);
+
----------------
chandlerc wrote:
> The comment isn't helping here. Again, please spell it 'SHA1' instead of 'Sha1'.
> 
> Also, array arguments with a size doesn't really make sense. They're just pointers in C++. I would either use a pointer or use a reference to an array of the given size.
Improved the comment, spelling and types. 

================
Comment at: lib/Fuzzer/FuzzerSha1.cpp:1-9
@@ +1,10 @@
+// This code is taken from
+// http://oauth.googlecode.com/svn/code/c/liboauth/src/sha1.c
+// and modified by adding 'namespace fuzzer {}' and an
+// interface function fuzzer::ComputeSha1().
+//
+/* This code is public-domain - it is based on libcrypt
+ * placed in the public domain by Wei Dai and other contributors.
+ */
+// gcc -Wall -DSHA1TEST -o sha1test sha1.c && ./sha1test
+
----------------
chandlerc wrote:
> Please use a normal LLVM style header and formatting. In particular please explain why this is here and not in lib/Support, etc.
done

================
Comment at: lib/Fuzzer/FuzzerSha1.cpp:14
@@ +13,3 @@
+
+namespace fuzzer {  // Added for LibFuzzer
+
----------------
chandlerc wrote:
> It would seem better to put this in an anonymous namespace and include the FuzzerInternal.h to define the ComputeSHA1 function in the fuzzer namespace.
done

================
Comment at: lib/Fuzzer/FuzzerSha1.cpp:221-225
@@ +220,7 @@
+
+/* self-test */
+
+#if SHA1TEST
+using namespace fuzzer;
+#include <stdio.h>
+
----------------
chandlerc wrote:
> I really don't like this approach. Why not just write a normal LLVM unittest for this (or other parts of the Fuzzer library)?
This is from the original code. 
I've removed this part and added a unit test to test/FuzzerUnittest.cpp

http://reviews.llvm.org/D9733

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list