On Wed, 11 Dec 2024, Pali Rohár wrote:

On Wednesday 11 December 2024 17:43:40 Martin Storsjö wrote:
On Wed, 11 Dec 2024, LIU Hao wrote:

在 2024-12-11 21:33, Martin Storsjö 写道:

  if (gPEDta)
    {
-      va_rel = 
gPEDta->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress;
-      sz_rel = 
gPEDta->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].Size;
+      if (gPEDta->OptionalHeader.NumberOfRvaAndSizes >
IMAGE_DIRECTORY_ENTRY_BASERELOC)
+        {
+          va_rel = gPEDta- 
>OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress;
+          sz_rel = 
gPEDta->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].Size;
+        }
    }

For these changes, isn't the > comparison wrong? Wouldn't it be
enough if we have OptionalHeader.NumberOfRvaAndSizes >=
IMAGE_DIRECTORY_ENTRY_BASERELOC?

I think this should be a normal `index < size` pattern, but written
backwards as `size > index`. So it's not incorrect, but weird.

Thanks, you're right, sorry for the mixup.

The curiosity about imports being handled only for one of PE and PE+ would
be nice to check though.

// Martin

The idea of the code is:

To access DataDirectory[0] it is required that number of directories
is more than 0. If there is zero number of directories then you cannot
dereference DataDirectory at offset zero.

I often write if-condition with variable at left side and constant on
the right side, hence in this case as:

 if (gPEDta->OptionalHeader.NumberOfRvaAndSizes > 
IMAGE_DIRECTORY_ENTRY_BASERELOC)
   va_rel = 
gPEDta->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress;

But if you prefer constant on the left side, you can rewrite condition as:

 if (IMAGE_DIRECTORY_ENTRY_BASERELOC < 
gPEDta->OptionalHeader.NumberOfRvaAndSizes)
   va_rel = 
gPEDta->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress;

No need to rewrie that, even though I guess that form would maybe feel more usual - but I guess it's also true that the usual form of comparisons is "(variable) (operator) (constant).

Anyway, this is fine, and with the explanation about the assymetrical use of the import directory, this is fine, so I pushed it.

// Martin

_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public

Reply via email to