This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 1/5] Import "glob" and "getcwd" modules from gnulib


On 09/21/2017 12:12 AM, Sergio Durigan Junior wrote:
> On Wednesday, September 20 2017, Pedro Alves wrote:
> 
>> (There's no need of wrap #undef with #ifdef/#endif.  That's redundant.)
>>
>> That'd #undef 'close' on all hosts, even if gnulib decides to
>> replace it for some reason.  E.g., REPLACE_FCHDIR
>> check in rpl_close (see my previous email).
> 
> Not all hosts; only on hosts that define USE_WIN32API.  This would
> basically make sure we stick to the current behaviour, which is to
> always define "close" as "closesocket" on win32.

Ah, this is all wrapped in #ifdef USE_WIN32API.  Missed that,
somehow.

> 
>> How about we switch close/closesocket around:
>>
>> #ifndef USE_WIN32API
>> # define closesocket close
>> #endif
>>
>> And then use closesocket instead of close?
> 
> That'd work, but my preference is to use "close" everywhere because
> that's the de facto way of dealing with sockets.  Only win32 hosts need
> this "closesocket" thing, and I don't think it makes sense to base use
> this name in our sources.
> 
> But that's my opinion; if you want, I can reverse the logic on the
> define's.

I don't see the problem.  Imagine it as if struct serial had
been C++-ified already, making struct ser_tcp a class, and we
added a private method like this:

void
ser_tcp::closesocket ()
{
#ifdef USE_WIN32API
   ::closesocket (fd);
#else
   close (fd);
#endif
}

Then we'd be calling closesocket instead of close.  :-)

Actually, that's pretty much the existing net_close, and
we call it in several places instead of close directly
in very nearby code:

...
	    close (scb->fd);
	    goto retry;
	  }
	if (err)
	  errno = err;
	net_close (scb);
...

Eh.

But I digress...

The #undef close approach is fine with me too
since it turns out that's wrapped in #if USE_WIN32API,
and I missed it the first time.

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]