[squid-dev] [PATCH] Correct various problems with errno

[Amos Jeffries]

On 8/05/2015 5:53 p.m., Amos Jeffries wrote:
> On 1/05/2015 5:48 p.m., Alex Dowad wrote:
>> Dear Squid devs,
>>
>> Please have a look at the attached patch. Your feedback will be appreciated.
>>
> 
> Sorry for the lag. Looks better than before.
> 
> NP: I'm holding off on approving until I can clear up whats happened to
> IPv6 in 3.5.4. I'm pretty certain its a side effect of the errno changes
> altering Squids state assumptions when IPv6 fails.
> 

Okay. Christos has resolved that now.


in src/comm/Read.cc:

* guidelines for errno usage state that it needs to be cleared (
errno=0) before the function being monitored so that values from
previous syscalls do not leak across.
 - so the line errno = 0; before FD_READ_METHOD needs to be retained.

* xerrno can be initialized just before its needed, and at the same time
errno is reset. like so:
  int xerrno = errno = 0;

* given the above two changes the redundant if(retval < 0) wrapping
xerrno assignment can be removed. Just copy the errno to xerrno
unconditionally.
 - we treat xerrno as if it were errno from that point onwards. The
requirements of only checking errno if there was an error transfer to
xerrno.


in src/comm/TcpAcceptor.cc:

* there is a reset of errno=0 which is still missing and not

* the use of xstrerror() needs converting to xstrerr(errcode)


in src/comm/Write.cc:

* follow the same pattern as for src/comm/Read.cc with xerrno
initialized at the last moment and when resetting errno, errno copied to
it unconditionally, etc.

* also xstrerror() repalcement with xstrerr(xerrno) in the relevant
debugs lines
 - the ' << "."' suffix output can be dropped on those lines as well
 - (plus opportunistic removal of HERE macros on the changed lines.)


Also, I await answer on my earlier question of whether you were
interested / intending to do a larger (full?) audit of errno usage and
removal of xstrerror() ?
 I would like to get work on that underway ASAP, if you are not
intending to take it on I would like to know so I can start it myself.

Cheers
Amos