[PATCH] D60042: [llvm-objcopy] Add --prefix-alloc-sections

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 2 23:02:22 PDT 2019


jakehehrlich requested changes to this revision.
jakehehrlich added a comment.
This revision now requires changes to proceed.

I'm not sure I like renaming and I'm not sure anyone requires that behavior. It also introduces n^2 behavior which I don't like. That should be removed at the very least.

Also don't we already have an update sections loop somewhere in this already? I think this should go there to reduce the number of passes we make over the sections



================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:612
 
+  if (!Config.AllocSectionsPrefix.empty()) {
+    for (auto &Sec : Obj.sections()) {
----------------
rupprecht wrote:
> Typical llvm style is to only include braces if there is more than one statement, i.e. this should just be:
> 
> ```
>   if (!Config.AllocSectionsPrefix.empty())
>     for (auto &Sec : Obj.sections())
>       if ((Sec.Flags & SHF_ALLOC) != 0)
>         Sec.Name = (Config.AllocSectionsPrefix + Sec.Name).str();
> ```
Do we follow that even for such deep nestings? I think I've only been doing it 2 layers deep before I add brackets to the outer most block.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60042/new/

https://reviews.llvm.org/D60042





More information about the llvm-commits mailing list