-
Notifications
You must be signed in to change notification settings - Fork 671
apps/net: add http connectivity check API #3304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Add netlib_check_httpconnectivity() to verify HTTP service connectivity by sending GET request and validating status code. Signed-off-by: meijian <[email protected]>
|
Documentation supplement here |
| * dest The location to return the IPv4 address. | ||
| * | ||
| * Return: | ||
| * 0 on success; a nagtive on failure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zs39 please fix the typo: nagtive -> negative
cederom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @zs39 :-)
- Are you sure use or
return -errno;is always valid? Please take a look at code comments. I think there are functions that does not set globalerrnoand thus return value may be undefined? - Would it be possible to add example code from the PR to
apps/examplesplease? :-)
|
|
||
| if (getaddrinfo(hostname, NULL, &hint, &info) != OK) | ||
| { | ||
| return -errno; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cederom the usage is right, please read https://man7.org/linux/man-pages/man3/errno.3.html, to understand when errno get set.
|
|
||
| if (ret < 0) | ||
| { | ||
| return -errno; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar as above why not return ret ?
I am putting this discussion here so it does not disappear after code comments are closed ;-) @zs39 please answer my questions about errno handling:
Lets take a looks at your proposition You are not getting and returning code of 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): I cannot see If you look at existing code use of
Once again, your implementation is prone to returning I would advise update to something like existing examples (1,2,3) above use: The only valid use of [1] https://man.freebsd.org/cgi/man.cgi?query=socket&apropos=0&sektion=0 |
Summary
Add netlib_check_httpconnectivity() to verify HTTP service connectivity by sending GET request and validating status code.
Impact
The new features have no impact.
Testing
Use the demo below for testing. Test passed.
The results are as follows