[PATCH] D118793: [COFF] Move section name encoding into BinaryFormat
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 2 23:27:23 PST 2022
jhenderson accepted this revision.
jhenderson added a comment.
Some nits from me. Otherwise, LGTM.
================
Comment at: llvm/include/llvm/BinaryFormat/COFF.h:735
+/// Encode section name based on string table offset.
+/// The size of Out must be at least NameSize.
+bool encodeSectionName(char *Out, uint64_t Offset);
----------------
`NameSize` isn't referenced anywhere in this signature - I have to go a long way to find what it refers to. Perhaps replace `NameSize` with `the NameSize constant value` or something to that effect?
================
Comment at: llvm/lib/BinaryFormat/CMakeLists.txt:14
XCOFF.cpp
+ COFF.cpp
----------------
Nit: filenames look to be in alphabetical order in this list, so move COFF.cpp much earlier.
================
Comment at: llvm/lib/BinaryFormat/COFF.cpp:1
+//===- llvm/BinaryFormat/COFF.cpp - The COFF format -------------*- C++ -*-===//
+//
----------------
Nit: No need for the "*- C++-*" bit of this line - this is only for .h files where the extension doesn't signify the language.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118793/new/
https://reviews.llvm.org/D118793
More information about the llvm-commits
mailing list