Page MenuHomePhabricator

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

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

Diff Detail


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? ;)

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.

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: - 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:

echristo accepted this revision.Jun 14 2018, 1:16 PM
echristo added inline comments.
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
3226 ↗(On Diff #150642)

Thank you for reviewing @dblaikie @echristo