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

[Brian P. <br...@sg...>]

> -----Original Message-----
> From: rea...@li...
[mailto:reactos-kernel-
> ad...@li...] On Behalf Of Casper Hornstrup
> Sent: Wednesday, June 12, 2002 7:34 AM
> To: rea...@li...
> Subject: RE: [ros-kernel] Patch: Teach freeldr registry about
> multiandexpanded types
> 
> 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).

This is incorrect. ValueBuffer is a pointer. This is how you can take a
regular pointer and use it like it was an array. So you are passing in
the address of the pointer to the array. Try printing the values of the
two addresses and you will see that they are different.

> > 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.

Again, BiosMemoryMap is just a pointer, so all you are putting on the
stack is 4 bytes. The compiler actually interprets a parameter such as
this to be just a pointer. That's why if you use sizeof() on a function
parameter that is an array it will only return 4 instead of the size of
the array. If you run lint on code that does this it will throw an error
at you.

> > 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.

I think I will just create a structure called BIOS_MEMORY_MAP_ARRAY. It
will have one member of type BIOS_MEMORY_MAP which is an array of 32
items. And I'll change this function to take a PBIOS_MEMORY_MAP_ARRAY
pointer as a parameter. What do you think?

> >
> > 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
> 
> 
> 
> _______________________________________________________________
> 
> Sponsored by:
> ThinkGeek at http://www.ThinkGeek.com/
> _______________________________________________
> reactos-kernel mailing list
> rea...@li...
> https://lists.sourceforge.net/lists/listinfo/reactos-kernel