Re: [RFA/win32] Avoid a couple of name collisions in win32-nat.c

[Christopher Faylor <cgf-use-the-mailinglist-please at sourceware dot org>]

On Wed, Jan 14, 2009 at 10:22:28AM +0400, Joel Brobecker wrote:
>> > >2009-01-07  Joel Brobecker  <brobecker@adacore.com>
>> > >
>> > >        * win32-nat.c (kernel32_DebugSetProcessKillOnExit): Renames
>> > >        DebugSetProcessKillOnExit.  Update all uses in this file.
>> > >        (kernel32_DebugActiveProcessStop): Renames DebugActiveProcessStop.
>> > >        Update all uses in this file.
>> 
>> This one is now in. I will review ASAP all the names we are using
>> when importing routines from DLLs, to see if there are others that
>> might need an adjustment.
>
>Just for the record, Chris asked:
>
>| Yes.  I guess that means that the rename to prefix kernel32_ is approved
>| too but I'd appreciate it if you would universally add the kernel32_ prefix
>| to everything that is dynamically derived from kernel32.
>
>I double-checked every call to GetProcAddress, and the kernel32_
>prefix is already added to all the function pointers related to
>kernel32 routines.
>
>There are a bunch of function pointers whose name is still identical
>to the function name:
>
>  static BOOL WINAPI (*OpenProcessToken)(HANDLE, DWORD, PHANDLE);
>  static BOOL WINAPI (*LookupPrivilegeValue)(LPCSTR, LPCSTR, PLUID);
>  static BOOL WINAPI (*AdjustTokenPrivileges)(HANDLE, BOOL, PTOKEN_PRIVILEGES,
>                                              DWORD, PTOKEN_PRIVILEGES, PDWORD);
>
>But these routines are from advapi32.dll, and are declared inside
>a function (set_process_priviledge).
>
>I don't think you really wanted me to rename this variables too,
>since local variables should not cause a collision (they should just
>hide the global names, if any). And because everything is local,
>the addition of the advapi32_ prefix does make the reading of the
>code a little harder, IMO. But just in case, here is the corresponding
>patch.  It's untested for now, but I can test it before checking in.
>It does compile, though.
>
>2009-01-14  Joel Brobecker  <brobecker@adacore.com>
>
>        * windows-nat.c (set_process_privilege): Rename OpenProcessToken,
>        LookupPrivilegeValue and AdjustTokenPrivileges by prefixing them
>        with "advapi32_". Adjust the code accordingly.

I think you're right.  It does make the code harder to read.  I've been toying
with putting guards around the windows header files which define the other
functions so that they can just be used like you would normally without the
kernel32_ prefix but I haven't been able to convince myself that this would
be a big improvement.

I don't think we need the advapi32_ here though since it isn't causing problems.

Thanks for researching this.

cgf