Re: [PATCH] mingw: redefine the wrapper macro after the corresponding function

[Karsten Blees <karsten.blees@gmail.com>]

Am 05.06.2014 18:56, schrieb Johannes Sixt:
> Am 05.06.2014 10:05, schrieb Stepan Kasal:
>> mingw.c defines several wrapper functionsi, like mingw_unlink().
>> These wrappers are deployed by macros like this:
>> 	#define unlink mingw_unlink
>> The function itself is preceded by #undef, leaving the wrapper out
>> of the game for the rest of mingw.c.
>>
>> This was not probably intentional; for example, there are three
>> calls to open() below the definition mingw_open() that probably
>> have no reason to circumvent the wrapper.
>> OTOH, there is one call to gethostbyname() before it was undefined;
>> probably happy that it actually calls mingw_gethostbyname().
>>
>> This patch adds back the #define after each wrapper definition.
>>
>> Signed-off-by: Stepan Kasal <kasal@ucw.cz>
>> ---
>>  compat/mingw.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/compat/mingw.c b/compat/mingw.c
>> index a0e13bc..e7193c0 100644
>> --- a/compat/mingw.c
>> +++ b/compat/mingw.c
>> @@ -224,6 +224,7 @@ int mingw_unlink(const char *pathname)
>>  	       ret = unlink(pathname);
>>  	return ret;
>>  }
>> +#define unlink mingw_unlink
> (etc...)
> 
> I don't particularly like this approach: It robs the precise control of
> which function we can invoke from other places in mingw.c.
> 
> Within mingw.c, if some other function inside mingw.c wants to use
> mingw_unlink, then it should be written as 'mingw_unlink(foo)', not
> 'unlink(foo)'.
> 

I very much like this approach. In fact, we already do this for e.g. mingw_raise.

> So, IMO the macros should be #undef'ed at the top of the file, and all
> users (like the open() and gethostbyname() invocations that you
> identified) should be audited and changed to call the function they
> actually need (i.e., the system open vs. mingw_open).
> 

I'm sceptical of moving all #undef's to the top. Other callers would typically want the wrapped version (i.e. mingw_*). At least I can't think of a scenario in which a higher level function would want to bypass the wrapper...

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.