patacongo commented on a change in pull request #1047:
URL: https://github.com/apache/incubator-nuttx/pull/1047#discussion_r425379296
##########
File path: net/netdev/netdev_register.c
##########
@@ -333,6 +335,18 @@ int netdev_register(FAR struct net_driver_s *dev, enum
net_lltype_e lltype)
return -EINVAL;
}
+ /* Update the package length */
+
+ if (dev->d_llhdrlen == 0)
+ {
+ dev->d_llhdrlen = llhdrlen;
+ }
+
+ if (dev->d_pktsize == 0)
+ {
+ dev->d_pktsize = pktsize;
+ }
+
Review comment:
> This could break network drivers that do not initialilze
dev->d_llhdrlen and d_pktsize to zero. Did you verify that all network drivers
do this?
It would be good to have an explanation why you made this change. It does
not seem right, but I do not know the motivation. I guess this would only
occur if you regiser the network driver twice. If that is the case we should
document when and why you do that and shy the link layer protocol of only the
first registration is retained.
Do you think that you could add some comments explaining what this is for?
I would appreciate that.
I don't believe that there is any problem with uninitialized data in the
current network drivers. There should be no problem if the driver structure
instance is defined in .bss or if the driver structure instance is allocated by
kmm_zalloc(). That seems to be the case always:
#!/bin/sh
DRIVERS=`grep -rl net_driver_s *`
for driver in $DRIVERS; do
echo "=== $driver"
grep net_driver_s $driver | grep kmm_ | grep alloc | grep -v kmm_zalloc
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
