You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "jturnsek (via GitHub)" <gi...@apache.org> on 2023/03/05 19:40:35 UTC
[GitHub] [nuttx] jturnsek opened a new pull request, #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
jturnsek opened a new pull request, #8728:
URL: https://github.com/apache/nuttx/pull/8728
## Summary
Calling fcntl with O_NONBLOCK flag on local socket was not working without this change.
## Impact
local_ioctl function
## Testing
Working with nng library, I have found the problem and this hotfix has helped.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1126985173
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Ok, let's remove line 877-879 which doesn't need anymore.
BTW, is it a listen socket? it make sense that both in/out pipe doesn't exist. What's difference return OK not -ENOTTY.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1126929167
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
> Oh, in most case, lc_infile/lc_outfile shouldn't NULL. In your case, is both NULL?
Yes, Both are NULL. Here is my callstack:
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1125723004
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
@xiaoxiang781216 @anchao could you please take a look? I recall some changes around `FIONBIO` made some time ago related to default behavior.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1126996314
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Yes, is a listen socket. NNG code is calling (void) fcntl(fd, F_SETFL, O_NONBLOCK) to set the socket in non block mode. You can see from callstack above the call to fcntl. When local_ioctl returns ENOTTY, netdev_file_ioctl is called next, which is then responsible to set s_flags for the socket. In case of OK, this doesn't happen.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1125832708
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
it's already be done at line 878.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1126011166
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
I do not remember exactly, because it was quite some time I did the debugging. I will try again the nng example this evening with the latest code and see what happens.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1127318356
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Ok, the patch looks good, please remove the default case. BTW, https://github.com/apache/nuttx/pull/8751 will fix some problem and contain more context information.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1125967267
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
> it's already be done at line 878.
I do not think it is the same. If you see the first case FIONBIO, I think there is a way to go through this case without setting ret variable, thus ret was set to OK at init and this is not the proper return value in that case. Am I wrong here?
In nuttx-apps repo, I have submitted a PR for nng with Pub/Sub example and you can test it.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1125974886
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Oh, in most case, lc_infile/lc_outfile shouldn't NULL. In your case, is both NULL?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1126012200
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Thanks.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1128005013
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Do we close this PR?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#issuecomment-1458666299
Closing this PR in favor of #8751.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1127385043
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Will test with NNG.
Yesterday, when testing NNG PubSub example, I have found out that example doesn't work as it was a few months ago. Although I am getting to nng_recv (before that ENOTTY code blocked in dial) function in client thread, nothing seems to get through, altough server thread is pushing messages. Something must have been changed in local socket implementation?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1125974886
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Oh, in most case, lc_infile/lc_outfile shouldn't NULL.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1128218226
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
yes, if my patch fix your problem.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1127318356
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Ok, the patch looks good, please remove the default case. BTW, https://github.com/apache/nuttx/pull/8751 will address the problem.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1126940782
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
@xiaoxiang781216 so should we remove the assignment in the default case?
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
@xiaoxiang781216 so should we remove the assignment in the `default` case?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1127318356
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Ok, the patch looks good, please remove the default case. BTW, https://github.com/apache/nuttx/pull/8751 will fix some problem.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] jturnsek closed pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "jturnsek (via GitHub)" <gi...@apache.org>.
jturnsek closed pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
URL: https://github.com/apache/nuttx/pull/8728
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8728: Hotfix of local_ioctl for fcntl with O_NONBLOCK to work
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8728:
URL: https://github.com/apache/nuttx/pull/8728#discussion_r1127318356
##########
net/local/local_sockif.c:
##########
@@ -833,7 +833,7 @@ static int local_close(FAR struct socket *psock)
static int local_ioctl(FAR struct socket *psock, int cmd, unsigned long arg)
{
FAR struct local_conn_s *conn;
- int ret = OK;
+ int ret = -ENOTTY;
Review Comment:
Ok, unterstand. https://github.com/apache/nuttx/pull/8751 should fix your problem and contain more context information.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org