[OpenSIPS-Devel] Improved socket error messages

[Michael Schloh von Bennewitz]

Hello Saul and Bogdan,

On Mon., Jan. 28, 2013, Bogdan-Andrei Iancu wrote:
>On Jan 28, 2013, Michael Schloh von Bennewitz wrote:
>>On Mon., Jan. 28, 2013, Saúl Ibarra Corretgé wrote:
>>>On Jan 27, 2013, at 8:17 PM, Michael Schloh von Bennewitz wrote:
>>>>  Jan 02 18:18:02 hostname <error> opensips[1234]: ERROR:core:tcp_blocking_connect: addr (1.2.3.4) port (98765), failed to retrieve SO_ERROR (146) Connection refused
>>>> 
>>>> ...which of course is much more useful.
>>>> 
>>>> [...]
>>>> 
>>>> -				LM_ERR("failed to retrieve SO_ERROR (%d) %s\n", err,
>>>> +				LM_ERR("addr (%s) port (%d), failed to retrieve SO_ERROR (%d) %s\n", inet_ntoa(((struct sockaddr_in *)servaddr)->sin_addr), ntohs(((struct sockaddr_in *)servaddr)->sin_port), err,
>>>> 						strerror(err));
>>>> 				goto error;
>>>> 			}
>>>>
>>>Improved logging messages are of course great :-) I have a few unfinished patches in this regard as well, so if you send over partial improvements I'll incorporate them. However, the patch you sent is not correct because it doesn't deal with IPv6 addresses, you need to check the sa_family field of the sockaddr struct before casting it to sockaddr_in or sockaddr_in6.
>>>
>>Wow, that's quite embarassing considering that I'm running IPv6
>>myself. Here are two equivalent corrected versions of the hack.
>>You probably want the second which is easier to read.
>>
>>diff -Nau tcp_main.c.orig tcp_main.c
>>--- tcp_main.c.orig	2013-01-27 19:35:31.898051715 +0100
>>+++ tcp_main.c	2013-01-28 18:30:27.516978242 +0100
>>@@ -342,7 +342,8 @@
>> 			getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &err_len);
>> 			if ((err==0) && (poll_err==0)) goto end;
>> 			if (err!=EINPROGRESS && err!=EALREADY){
>>-				LM_ERR("failed to retrieve SO_ERROR (%d) %s\n", err,
>>+				char erraddrstr[INET6_ADDRSTRLEN];
>>+				LM_ERR("addr (%s) port (%d), failed to retrieve SO_ERROR (%d) %s\n", servaddr->sa_family == AF_INET ? inet_ntoa(((struct sockaddr_in *)servaddr)->sin_addr) : inet_ntop(AF_INET6, &((struct sockaddr_in6 *)servaddr)->sin6_addr, erraddrstr, sizeof(erraddrstr)), servaddr->sa_family == AF_INET ? ntohs(((struct sockaddr_in *)servaddr)->sin_port) : ntohs(((struct sockaddr_in6 *)servaddr)->sin6_port), err,
>> 						strerror(err));
>> 				goto error;
>> 			}
>>
>>diff -Nau tcp_main.c.orig tcp_main.c
>>--- tcp_main.c.orig	2013-01-27 19:35:31.898051715 +0100
>>+++ tcp_main.c	2013-01-28 18:35:23.761388145 +0100
>>@@ -342,8 +342,11 @@
>> 			getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &err_len);
>> 			if ((err==0) && (poll_err==0)) goto end;
>> 			if (err!=EINPROGRESS && err!=EALREADY){
>>-				LM_ERR("failed to retrieve SO_ERROR (%d) %s\n", err,
>>-						strerror(err));
>>+				char erraddrstr[INET6_ADDRSTRLEN];
>>+				if (servaddr->sa_family == AF_INET)
>>+					LM_ERR("addr (%s) port (%d), failed to retrieve SO_ERROR (%d) %s\n", inet_ntoa(((struct sockaddr_in *)servaddr)->sin_addr), ntohs(((struct sockaddr_in *)servaddr)->sin_port), err, strerror(err));
>>
>See an improved version of your patch here on commit #9666 :
>    http://opensips.svn.sourceforge.net/opensips/?rev=9666&view=rev
>
>If you consider adding that extra info in other places, please
>provide a patch.
>
Your correction is better than my correction so thanks alot for
providing it and making the commit. I'll pull out my POSIX version
of the correction [1] once 1.9.0 is out and my specs are updated.

I'll keep an eye out for other places as well (surely
there are many.)

[1] http://scm.europalab.com/opkg/rev/a2c6460cfb16

Regards,
Michael