[PATCH] D34525: Replace trivial use of external rc.exe by writing our own .res file.
Eric Beckmann via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 22 15:50:16 PDT 2017
ecbeckmann added inline comments.
================
Comment at: lld/COFF/DriverUtils.cpp:424-426
+ // Strip all newlines from the Manifest.
+ Manifest.erase(std::remove(Manifest.begin(), Manifest.end(), '\n'),
+ Manifest.end());
----------------
ruiu wrote:
> Why do you want to strip all newlines?
This mimics the behavior of rc.exe, which strips all the newlines from a manifest before putting it in the .res file. However, spaces are left intact.
================
Comment at: lld/COFF/DriverUtils.cpp:430
+
+ char *Current = const_cast<char *>(Buf->getBufferStart());
+ writeResFileHeader(Current);
----------------
ruiu wrote:
> Rename Current Buf and use uint8_t instead of char for consistency
Hmmm there is already a variable named "Buf".
================
Comment at: lld/COFF/DriverUtils.cpp:430
+
+ char *Current = const_cast<char *>(Buf->getBufferStart());
+ writeResFileHeader(Current);
----------------
ecbeckmann wrote:
> ruiu wrote:
> > Rename Current Buf and use uint8_t instead of char for consistency
> Hmmm there is already a variable named "Buf".
Also const_char to type uint8_t* is not possible, and requires an additional cast.
================
Comment at: llvm/include/llvm/Object/WindowsResource.h:67-72
+struct WinResIDs {
+ const uint16_t TypeFlag = 0xffff;
+ support::ulittle16_t TypeID;
+ const uint16_t NameFlag = 0xffff;
+ support::ulittle16_t NameID;
+};
----------------
ruiu wrote:
> You are not really using this struct. If you need only the size of the struct, define it as a constant.
I do use in in DriverUtils.cpp for a reinterpret_cast. It helps I think for writing the resource header.
https://reviews.llvm.org/D34525
More information about the llvm-commits
mailing list