[llvm-commits] RFC: new tool: llvm-strings

[Sean Silva]

+error_code findStringsInBlob(StringRef theData, std::size_t initialOffset,
+          std::size_t sizeThreshold, bool verbose, PrintFunction pf) {

Please align this like the rest of the codebase.

+//  outs() << "Blob( " << theData.size() << ", " << initialOffset << ", "
+//      << sizeThreshold << ", pf)\n";

Don't leave commented-out code in your patches. Remove before submitting.

+  for ( ; p != end && !retVal; ++p ) {

check your spacing around parens to be like the rest of the codebase.

+    if ( isPrintable( *p )) {
+      if ( curr == NULL )
+        curr = p; // start new string
+      }

Please put your curly braces like the rest of the codebase and not
this bizarre "indent the end brace" thing you are doing.

+//  Display the current settings, if asked
+  if ( OptionVerbose ) {

The comment should be at the same indentation as the code it comments.

-- Sean Silva

On Tue, Nov 13, 2012 at 10:29 AM, Marshall Clow <mclow.lists at gmail.com> wrote:
> On Nov 12, 2012, at 12:58 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> On Mon, Nov 12, 2012 at 7:58 AM, Marshall Clow <mclow.lists at gmail.com>
> wrote:
>>
>> Here's a first cut at a "strings" tool for LLVM.
>>
>> It is currently missing one feature: the ability to dump the strings from
>> a single object file in an archive.
>> There are also some 80 column style violations in the source.
>>
>> I'll be fixing both of those and re-posting soon - but I'm posting now
>> this to get feedback - to see if anyone thinks that I'm missing (any other)
>> stuff.
>
>
> Tabs, oh god the tabs.
>
>
> To spare Eric the agony, I have:
> * removed all the tabs
> * cleaned up a bunch of style violations
> * implemented the feature to dump only the contents of one object file in an
> archive.
>
> Micheal wrote:
>
>  The only concern I have is structuring FindStringsInBlob so that
> implementing --encoding is easy.
>
>
> This comment sailed right by me, Michael :-(
>
> -- Marshall
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>