RE: [ros-kernel] Patch: Teach freeldr registry about multi andexpanded types

[Casper H. <ch...@us...>]

ons, 2002-06-12 kl. 05:44 skrev Brian Palmer:

These are some changes I made to remove the warnings. They are not part
of Joseph's patch.

> I have a few questions about this:
> 
> reactos.c line 193:
> rc = RegQueryValue(hGroupKey, "List", NULL, (PUCHAR)&ValueBuffer,
> &BufferSize);
> 
> Shouldn't that be:
> rc = RegQueryValue(hGroupKey, "List", NULL, (PUCHAR)ValueBuffer,
> &BufferSize);
> 

They are both okay due to the way the compiler handles char arrays. The
first makes more sence to me because it takes the address of the array
and then casts it to a PUCHAR (ie. it casts a pointer to another
pointer). The second casts a char[] array to PUCHAR (ie. an array to
pointer cast).

> And the same thing in meminit.c line 64. Since BiosMemoryMap is a
> pointer to the array, you are passing in the address of the pointer to
> the array and casting it to type PBIOS_MEMORY_MAP.
> 

No. BiosMemoryMap is on the stack in MmInitializeMemoryManager(),
defined as BIOS_MEMORY_MAP BiosMemoryMap[32]. It is not a pointer.
Wether it should be on the stack is debatable because it is retrieved
from the BIOS twice (from what I gather), once by the memory manager and
once by the ReactOS boot code.

> I also noticed that this patch changes all the function prototypes from
> ULONG GetBiosMemoryMap(BIOS_MEMORY_MAP BiosMemoryMap[32]); to ULONG
> GetBiosMemoryMap(PBIOS_MEMORY_MAP BiosMemoryMap); Now this would
> normally be fine except that that function returns an array of exactly
> 32 items since the size of the memory map is not known before the
> function call.

Yes that is a bit unsafe. Maybe we should rename BIOS_MEMORY_MAP to
BIOS_MEMORY_MAP_ENTRY and create a new type for the bios memory map?

typedef BIOS_MEMORY_MAP_ENTRY BIOS_MEMORY_MAP[32];
typedef BIOS_MEMORY_MAP *PBIOS_MEMORY_MAP;

That way we can avoid some type casts.

> 
> Also the %ld in the strings passed to printf() is not supported in the
> printf() function included with freeldr. It should probably be updated.

I did not notice that ;o(

> 
> Thanks for the patch, every little bit helps.
> 
> Brian