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

[Marshall Clow]

On Nov 13, 2012, at 12:40 PM, Sean Silva <silvas at purdue.edu> wrote:
> 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.
> +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.


Ok, I think I've got all the style guidelines.
I haven't heard any complaints about the code ;-)

Now I just need docs and tests.


-- Marshall

Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
        -- Yu Suzuki
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-strings.patch3
Type: application/octet-stream
Size: 12734 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121114/e3c1b9c5/attachment.obj>