[PATCH] D50463: [llvm-objcopy] Add --prefix-sections option

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added a comment.

I don't think I want this option to do anything more than literally add a prefix to all section names. It isn't clear that those precise semantics are being relied on anywhere and the behavior of making relocation names and not renaming symtab etc... is a result of how bfd reconstructs the output binary without consideration for what the user initially put in which is opposite the stated goal of llvm-objcopy.



================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:274
 
+static inline bool StartsWith(std::string S1, StringRef S2) {
+  return std::equal(S2.begin(), S2.end(), S1.begin());
----------------
jhenderson wrote:
> This should take a pair of StringRefs, not a std::string and a StringRef. And then you can use StringRef::startswith.
> 
> Also `inline` has no purpose here. Compilers generally ignore it when it comes to function inlining. It does have a completely separate purpose in header files though.
> 
> It should also be named with lower-case (@jakehehrlich  and I discussed this offline, and I think that somewhere we started following a non-LLVM style for functions, contrary to the style guide, and we'd like to get back to the style it should be).
+1 on all of these points actually. The lowercase thing is my fault but we should begin changing it now.


Repository:
  rL LLVM

https://reviews.llvm.org/D50463