cederom commented on PR #3304: URL: https://github.com/apache/nuttx-apps/pull/3304#issuecomment-3734796496
>> @cederom: Where is errno defined and what sets its value? >> >> Looking at https://github.com/apache/nuttx/blob/fac76746eae2afe91a5cb9948e3843596c9f941f/include/errno.h#L43 there is no `set_errno(e)` in the https://github.com/apache/nuttx/blob/fac76746eae2afe91a5cb9948e3843596c9f941f/libs/libc/netdb/lib_getaddrinfo.c#L133. >> >> If `getaddrinfo()` returns error code by return then why not this? >> >> ``` >> int ret = getaddrinfo(hostname, NULL, &hint, &info); >> id ( ret != OK ) return ret; >> ``` > @xiaoxiang781216: @cederom the usage is right, please read https://man7.org/linux/man-pages/man3/errno.3.html, to understand when errno get set. I am putting this discussion here so it does not disappear after code comments are closed ;-) @zs39 please answer my questions about errno handling: 1. Where is errno defined and what sets its value? There are places in `getaddrinfo()` that will return an error code but does not set `errno` with `set_errno()` that is required to use `errno` as valid error code that you may reference in the caller function. 2. If `getaddrinfo()` returns error code by return then why not `return ret` instead `return -errno`? Lets take a looks at your proposition `netutils/netlib/netlib_checkhttpconnectivity.c` line `76`: ``` if (getaddrinfo(hostname, NULL, &hint, &info) != OK) { return -errno; } ``` You are not getting and returning code of `getaddrinfo()`. Instead when `getaddrinfo()` fails you return `-errno` that does not reflect error code returned. Let's go to that function (https://github.com/apache/nuttx/blob/fac76746eae2afe91a5cb9948e3843596c9f941f/libs/libc/netdb/lib_getaddrinfo.c#L133) and see first check (https://github.com/apache/nuttx/blob/fac76746eae2afe91a5cb9948e3843596c9f941f/libs/libc/netdb/lib_getaddrinfo.c#L151): ``` if (hostname == NULL && servname == NULL) { return EAI_NONAME; } ``` I cannot see `set_errno()` here (https://github.com/apache/nuttx/blob/fac76746eae2afe91a5cb9948e3843596c9f941f/include/errno.h#L44) thus in the call above you will not forward `EAI_NONAME` but some random `errno` that was probably set before by something else. If you look at existing code use of `getaddrinfo()` in the apps (https://github.com/search?q=repo%3Aapache%2Fnuttx-apps%20getaddrinfo&type=code) there are use cases like: 1. Return `ERROR=-1` when `getaddrinfo()` fails, see: https://github.com/apache/nuttx-apps/blob/63f61d080aa524ed3c79518e678ddfa6dbe5a33c/netutils/ping/icmpv6_ping.c#L119 2. Convert `ret = getaddrinfo()` to get string error, see: https://github.com/apache/nuttx-apps/blob/63f61d080aa524ed3c79518e678ddfa6dbe5a33c/examples/ftpc/ftpc_main.c#L444 3. Both (1) and (2) above ret is used to get error strting and `ERROR=-1` is returned, see: https://github.com/apache/nuttx-apps/blob/63f61d080aa524ed3c79518e678ddfa6dbe5a33c/nshlib/nsh_netcmds.c#L1172 Once again, your implementation is prone to returning `errno` set somewhere else instead valid return code from `getaddrinfo()`. I would advise update to something like existing examples (1,2,3) above use: ``` int ret = getaddrinfo(hostname, NULL, &hint, &info); if ( ret != OK ) return ret; ``` The only valid use of `errno` may be for `socket()` [1] as it known to set it explicitly. But it's not for `getaddrinfo()` [2]. [1] https://man.freebsd.org/cgi/man.cgi?query=socket&apropos=0&sektion=0 [2] https://man.freebsd.org/cgi/man.cgi?query=getaddrinfo&apropos=0&sektion=3 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
