[PATCH] D56840: [llvm-objcopy] [COFF] Implement --only-keep-debug

[James Henderson via Phabricator via llvm-commits]

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