a-lunev edited a comment on pull request #4575:
URL: https://github.com/apache/incubator-nuttx/pull/4575#issuecomment-924272301
> @xiaoxiang781216 I tried to find if changing dev->d_flags is synchronized
to net_lock() anywhere. It looks like dev->d_flags can be modified (e.g. by
psock_ioctl()) asynchronously to net_lock() / net_unlock critical sections.
> In this case dev->d_flags may change its state to IFF_DOWN right after you
have checked (dev->d_flags & IFF_UP). Then the event callback will still be
added to the event list while the network adapter is already down.
> Is it correct?
@xiaoxiang781216 I've analyzed the source code once again. I've noticed that
for some reason devif_callback_free() is invoked with the second argument =
NULL:
`devif_callback_free(NULL, NULL, list_head, list_tail);`
The second argument of devif_callback_free() is "FAR struct devif_callback_s
*cb".
If the second argument = NULL, then devif_callback_free() does not do
anything and just returns. That will result in a memory leak as the callback
structure will not be released.
As I understand, the following correction is required:
```
git diff
diff --git a/net/devif/devif_callback.c b/net/devif/devif_callback.c
index 69a8b249c1..aa33695c23 100644
--- a/net/devif/devif_callback.c
+++ b/net/devif/devif_callback.c
@@ -283,7 +283,7 @@ FAR struct devif_callback_s *
{
/* No.. release the callback structure and fail */
- devif_callback_free(NULL, NULL, list_head, list_tail);
+ devif_callback_free(NULL, ret, list_head, list_tail);
net_unlock();
return 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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
