[PATCH] D58173: [llvm-objcopy] Add --set-start, --change-start, --adjust-start

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/change-entry-point.test:23
+
+# Test --change-start after --set-start. Result should be 0x1150
+# RUN: llvm-objcopy --set-start 0x1000 --change-start 0x100 --change-start 0x50 %t %t11
----------------
Nit: trailing full stop, here and on the other comments in this test.


================
Comment at: tools/llvm-objcopy/CopyConfig.cpp:572
+      auto Expr = Config.EntryExpr ? std::move(Config.EntryExpr)
+                                   : [](uint64_t A) { return A; };
+      Config.EntryExpr = [Expr, EIncr](uint64_t EAddr) {
----------------
Could you just make this the default? I.e. set Config.EntryExpr to return the input unless something overrides it?


================
Comment at: tools/llvm-objcopy/CopyConfig.h:106
+  // ELF entry point address expression. Input parameter is an entry point
+  // address in the input ELF file. Entry address in the output file is
+  // calculated with EntryExpr(input_address), when either --set-start or
----------------
You still are missing the first "the" in the third sentence, as requested. Also, it should be "The input parameter is..." not just "Input parameter is..."


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

https://reviews.llvm.org/D58173