[PATCH] D33180: Add functionality to cvtres to parse all entries in res file.

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 19 11:39:34 PDT 2017


zturner added inline comments.


================
Comment at: llvm/lib/Object/WindowsResource.cpp:64-71
+  uint32_t Offset = DataSize + HeaderSize;
+
+  // Correct for the fact we already read 2 integers.
+  Offset -= 2 * sizeof(uint32_t);
+
+  // Advance and align.
+  RETURN_IF_ERROR(Reader.skip(Offset));
----------------
zturner wrote:
> Maybe I'm missing something, but I don't think any of these lines are needed?  You're computing `Offset` here by adding together the sizes of the two fields you just wrote.  Then subtracting the size of the two fields you just wrote, at which point `Offset` is going to be 0.  So you're skipping zero bytes, then padding to 4 byte alignment, but since you just read 2 uint32s, it should already be 4 byte aligned.
> 
> I think you can just delete all of this?
Ahh I think I follow now.  But why are you just reading the data and header size, but not the *actual* data or headers?


https://reviews.llvm.org/D33180





More information about the llvm-commits mailing list