[llvm-commits] [PATCH] llvm-strings tool

[Eric Christopher]

  Also needs tons of comments. The top level functions should have comments and it'd be nice to have some number of blocks in the code with functions as well.


================
Comment at: llvm-strings/llvm-strings.cpp:19
@@ +18,3 @@
+#include "llvm/ADT/StringExtras.h"
+
+#include "llvm/Object/Archive.h"
----------------
No need for spaces here either...

================
Comment at: llvm-strings/llvm-strings.cpp:237
@@ +236,3 @@
+      if (!(RetVal = object::createBinary(aFile.take(), obj)))
+        if (object::ObjectFile *o = dyn_cast<object::ObjectFile>(obj.get()))
+          RetVal = findStringsInObjectFile(o, 0, OptionMinStringLength,
----------------
Lowercase 'o' is a very hard to read/reason about variable name. Ditto with any of the others similarly named.

================
Comment at: llvm-strings/llvm-strings.cpp:75
@@ +74,3 @@
+
+template <typename PrintFunction> error_code findStringsInObjectFile(
+    const object::ObjectFile *obj, std::size_t initialOffset,
----------------
The indenting here feels awkward. See what clang-format can do to the file?


http://llvm-reviews.chandlerc.com/D275