Re: [PATCH] Re: errcode hides Comm::ListenStateData::errcode.

On 08/03/2010 02:33 AM, Amos Jeffries wrote:

> + errcode = errno = 0; // reset errno and local copy.
> if ((sock = accept(fd, gai->ai_addr, &gai->ai_addrlen)) < 0) {
>
> + errcode = errno; // store last accept errno locally.

I would not recommend resetting the system errno before the accept()
call. It might not be a writeable variable and resetting it is not
really needed. POSIX guarantees that errno will be [re]set if accept()
fails.

>> * The errcode member documentation ("errno code if any happened so
>> far") is inconsistent with the code because some errors are
>> ignored.
>>
>
> Fixed.

The patch does not modify errcode member documentation, AFAICT. I
suggest something like "errno from the last listen()/accept() failure or
zero".

* It still feels wrong that listen() failures are ignored. If ignoring
is the right thing to do, perhaps add a comment to explain why it is OK
to ignore them?

Please feel free to commit the patch even if you disagree with the above
comments -- your cleanup makes the code better overall regardless these
minor points, IMO.

Thank you,

Alex.
Received on Tue Aug 03 2010 - 16:56:45 MDT