[PATCH] D33566: Adding parsing ability for .res file, building the resource directory tree

[Eric Beckmann via Phabricator via llvm-commits]

ecbeckmann added inline comments.


================
Comment at: llvm/lib/Object/WindowsResource.cpp:80
   RETURN_IF_ERROR(Reader.readInteger(HeaderSize));
+
+  if (HeaderSize < MIN_HEADER_SIZE)
----------------
zturner wrote:
> I think this function would be a lot clearer if we had some structs that we could just read out in one or two lines rather than reading some fields, ignoring others, skipping sometimes, reading sometimes, etc.  How about this?
> 
> ```
> class ResourceEntryRef {
>   struct HeaderPrefix {
>     ulittle32_t DataSize;
>     ulittle32_t HeaderSize;
>     ulittle16_t Unknown;
>     ulittle16_t TypeID;
>     ulittle16_t IDFlag;
>     ulittle16_t NameID;
>   };
> 
>   struct HeaderSuffix {
>     ulittle32_t DataVersion;
>     ulittle16_t MemoryFlags;
>     ulittle16_t LanguageID;
>     ulittle32_t Version;
>     ulittle32_t Characteristics;
>   };
> 
>   const HeaderPrefix *Prefix = nullptr;
>   const HeaderSuffix *Suffix = nullptr;
> };
> 
> ...
> 
> Error ResourceEntryRef::loadNext() {
>   RETURN_IF_ERROR(Reader.readObject(Prefix));
>   if (Prefix->IDFlag != 0xFFFF) {
>     Reader.setOffset(Reader.getOffset() - 4);
>     RETURN_IF_ERROR(Reader.readWideString(Name));
>   }
> 
>   RETURN_IF_ERROR(Reader.readObject(Suffix));
>   RETURN_IF_ERROR(Reader.readArray(Data, Prefix->DataSize);
>   RETURN_IF_ERROR(Reader.padToAlignment(sizeof(uint32_t));
>   return Error::success();
> }
> ```
Hmmm this issue is actually pretty difficult because we can't really use a HeaderPrefix structure at all.  I didn't realize this when I first implemented this but actually both the Type and the Name can either be numeric IDs or variable length strings; therefore there isn't really any "standard" length that we can read into a structure easily.  Perhaps we can have a HeaderSuffix structure though....


https://reviews.llvm.org/D33566