[PATCH] D34525: Replace trivial use of external rc.exe by writing our own .res file.

[Eric Beckmann via Phabricator via llvm-commits]

ecbeckmann added inline comments.


================
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:
> ecbeckmann wrote:
> > 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.
> Then maybe you want to remove the default value 0xffff. You are not using it.
Oh shoot you're right.  These fields //must// be set to 0xffff when the Type and Names are IDs, however.  I should make a method for setting them.


================
Comment at: llvm/unittests/BinaryFormat/TestFileMagic.cpp:79
     "\xfe\xed\xfa\xce........\x00\x00\x00\x0b............";
-const char windows_resource[] = "\x00\x00\x00\x00\x020\x00\x00\x00\xff";
+const char windows_resource[] = "\x00\x00\x00\x00\x020\x00\x00\x00\xff\xff\x00\x00\xff\xff\x00\x00";
 const char macho_dynamically_linked_shared_lib_stub[] =
----------------
zturner wrote:
> This is modified but no test is modified.  Is that normal?
The change was as follows: I added the WinResMagic constant definition in BinaryFormat/COFF.h.  I then modified Magic.cpp to compare based on this constant instead of it's previous method of comparing to a string which, in fact, did not contain all the magic bytes.  This broke the test, because the test magic didn't contain the full signature, so it would fail the comparison in Magic.cpp


https://reviews.llvm.org/D34525