[PATCH] D56840: [llvm-objcopy] [COFF] Implement --only-keep-debug
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 18 06:17:18 PST 2019
jhenderson added inline comments.
================
Comment at: test/tools/llvm-objcopy/COFF/Inputs/only-keep-sections.yaml:1
+--- !COFF
+OptionalHeader:
----------------
If this yaml file is going to be used in other tests, it should be more generically named (unless the other tests also involve "only-keep-sections").
================
Comment at: test/tools/llvm-objcopy/COFF/only-keep-debug.test:7-9
+Check that all non-debug/buildid sections are truncated, but only ones
+that actually had IMAGE_SCN_CNT_CODE or IMAGE_SCN_CNT_INITIALIZED_DATA
+set.
----------------
This comment is a little clunky. Perhaps rephrase it to something like:
Check that all non-debug/buildid sections with IMAGE_SCN_CNT_CODE or IMAGE_SCN_CNT_INITIALIZED_DATA are truncated, and none others.
================
Comment at: tools/llvm-objcopy/COFF/COFFObjcopy.cpp:51-53
+ // For --only-keep-debug, we keep all other sections, but remove
+ // their content. The VirtualSize field in the section header
+ // is kept intact.
----------------
Nit of nits: don't shorten comment lines unnecessarily. clang-format will insert new lines to make them conform to the character width, but won't make them longer if they are unnecessarily short.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56840/new/
https://reviews.llvm.org/D56840
More information about the llvm-commits
mailing list