Page MenuHomePhabricator

[llvm-readobj] Add -string-dump (-p) option
ClosedPublic

Authored by paulsemel on Jun 9 2018, 3:52 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

paulsemel updated this revision to Diff 150642.Jun 9 2018, 3:52 PM
paulsemel created this revision.

Wrong diff. Updated to right one.

One inline nit and adding Dave here.

Dave: I put a note in the review here - as someone who wasn't originally a C programmer any ideas on how to do this more idiomatically? ;)

tools/llvm-readobj/ELFDumper.cpp
152 ↗(On Diff #150642)

Nit: Could we match the naming convention of the rest of the code?

3226 ↗(On Diff #150642)

Dave: Here.

gah, sorry - failed to submit my comment.

tools/llvm-readobj/ELFDumper.cpp
3226 ↗(On Diff #150642)

@echristo (neat - you can tag people in comments :) ) - yeah, looked around a bit in LLVM to see if we had something & nothing stood out (admittedly it's possible the function doesn't have "parseInt" as a substring - though all our actual (asm of various flavours/contexts, linker scripts, etc) parsers do have that in the name, but they're hand rolled for better error handling or supporting different radix prefixes, etc).

Seems C++11 has this for std::string: http://en.cppreference.com/w/cpp/string/basic_string/stol - but if you want to parse a chunk of characters not necessarily in a std::string, you might be out of luck for something more modern until C++17: http://en.cppreference.com/w/cpp/utility/from_chars

echristo accepted this revision.Jun 14 2018, 1:16 PM
echristo added inline comments.
tools/llvm-readobj/ELFDumper.cpp
3226 ↗(On Diff #150642)

Oh well :)

Thanks @dblaikie!

This revision is now accepted and ready to land.Jun 14 2018, 1:16 PM
This revision was automatically updated to reflect the committed changes.
paulsemel added inline comments.Jun 15 2018, 7:21 AM
tools/llvm-readobj/ELFDumper.cpp
3226 ↗(On Diff #150642)

Thank you for reviewing @dblaikie @echristo