[PATCH] D49870: [llvm-objcopy] Add support for --rename-section flags from gnu objcopy

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 26 16:57:25 PDT 2018


alexshap added inline comments.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:196
 
+enum SectionFlag {
+  SecNone = 0,
----------------
rupprecht wrote:
> alexshap wrote:
> > I think in any case it should not be in the global namespace,
> > but more importantly, probably it would be better to use ELF* constants directly rather than introducing this enum.
> > I think in any case it should not be in the global namespace,
> Moved to the anonymous namespace above (w/ config classes)
> 
> > but more importantly, probably it would be better to use ELF* constants directly rather than introducing this enum.
> 
> This isn't meant to replace the ELF SHF_* constants, it's meant to map to allowed command-line flags.
> For instance, "readonly" maps to "unset SHF_WRITE"
yea, it's not a replacement, but why do we need this double indirection - 
i mean now there is a switch (lines 215 - 228) and there is a chain of "if" statements (lines 255 - 265) -
it looks like the latter would be kinda redundant if ParseSectionName returned the flag (instead of the corresponding enum value) - am i missing smth ?


Repository:
  rL LLVM

https://reviews.llvm.org/D49870





More information about the llvm-commits mailing list