Re: [lwip-devel] [lwip-users] sock_set_errno semantics in lwip 2.1.2

[address@hidden]

Am 19.05.2021 um 13:27 schrieb Ruediger_Asche@t-online.de:
>
> Hi there Simon,
>
> thanks for your timely answer!
>
> I'm sorry, but this can NOT be the solution. First of all, a global variable 
> to carry shared error conditions was a pretty bad idea when it was incepted, 
> but in these days where even the smallest of systems employs 
> multithreading/processing and/or multiple communication channels, you can't 
> get any more stone age than falling back to a global errno.
>
> Second, threads and sockets are per se not related. There is nothing that 
> prevents a process to serve multiple sockets; thus, on thread local storage, 
> you would have to keep instance data for each socket. Likewise, even though 
> (at least in 1.4.1), sharing sockets among tasks was not supported, it can be 
> done under certain conditions. In those cases TLS (which FreeRTOS does NOT 
> support, by the way) wouldn't help either.
>
> In short, a socket based error code MUST be kept in the socket. Everything 
> else won't work reliably in any multitasking environment.
>
> I don't know why it was considered a bug in 1.4.1. It worked well for several 
> of my customers in hundreds of installations in the field, and it was the 
> right thing to do.
>
> I don't mind recoding for 2.x, but there MUST be a socket local error code to 
> maintain. If that doesn't appear to be possible, we must abandon lwip.

The getsockopt(SO_ERROR) and the socket to errno interface is pretty
well defined in the standards, and we try to follow standards. There
have been multiple requests since 1.4.x to better implement this
standard and that's what we did.

The errno variable is set for errors during a call (i.e. if a call
returns != success, check errno). For asynchronous errors, there's still
getsockopt(SO_ERROR). If there's anything missing in these asynchronous
paths which prevents you reading such an error, please file a bug report.

You are the first wanting to go back to the 1.4.1 behaviour though (of
recording *all* errors in the socket instead of errno). Maybe you want
to consider this before trying to persist in such strong words that only
your view on it is correct.

Regards,
Simon

>
>
> Am 19.05.2021 um 12:01 schrieb Ruediger_Asche@t-online.de:
>> Hi there, we have been using lwip in a FreeRTOS based firmware for many
>> years now and are upgrading from lwip 1.4.1 to 2.1.2.
>>
>>  
>>
>> We are using the following (pseudo)code to encapsulate recv() in a C++
>> class:
>>
>>  
>>
>> do
>> {
>>     int aCurrentCount = recv(<socket instance>,<ptr into recv
>> buffer>,aCharsRequested,theTimeout?MSG_DONTWAIT:0);
>>     if (aCurrentCount>0)
>>     {
>>         <there were characters received from the socket, handle>
>>     }
>>     else
>>     {
>>         socklen_t aReturnSize = sizeof(aError);
>>         int aError;
>>         getsockopt(<socket instance>, SOL_SOCKET, SO_ERROR,(void
>> *)&aError,&aReturnSize);
>>         if (!aCurrentCount || (aCurrentCount<0 && (aError!= EWOULDBLOCK)))
>>         {
>>             < connection gone down, close the socket, return failure or
>> partial read if applicable>
>>         }
>>     }
>> } < until timeout occurred or all requested characters have been received>
>>
>>  
>>
>> On 1.4.1 this worked fine. On 2.1.2, every recv() falls into the close
>> socket case.
>>
>>  
>>
>> I debugged this and found that in 2.1.2, sock_set_errno() has been
>> rewritten to not set the connection specific error code member
>> pending_err but the global variable errno.
>>
>> The getsockopt funtion, though, retrieves the pending_err member which
>> hasn't been set by sock_set_errno.
>>
>>  
>>
>> Needless to say, in a multi tasking, multiple socket environment it is
>> not feasible to use a global variable for connection specific errors, so
>> I can't look at the global errno to determine the failure code for this
>> socket.
>>
>>  
>>
>> 1.4.1 implemented the sock_set_errno macro to store the error in a
>> socket specific variable.
>>
>>  
>>
>> If there is a thread safe and per-socket way to implement the above
>> sequence, how do I need to rewrite the code? If not, I consider the
>> implementation a bug, how to report this?
>
> The actual bug was in 1.4.1 to not store such error codes in 'errno'.
> The socket API is not under our control, we try to stay to the opengroup
> spec as close as possible.
>
> Of course, you cannot use a simple global variable for 'errno' when
> using multithreading. You need something like "thread local storage",
> which I believe FreeRTOS should support.
>
> Regards,
> Simon
> 
>