netdev
[Top] [All Lists]

Re: [PATCH 2/4] [NETLINK] introduce netlink_check_skb function

To: Thomas Graf <tgraf@xxxxxxx>
Subject: Re: [PATCH 2/4] [NETLINK] introduce netlink_check_skb function
From: Pablo Neira <pablo@xxxxxxxxxxx>
Date: Sat, 12 Feb 2005 22:18:59 +0100
Cc: netdev@xxxxxxxxxxx, "David S. Miller" <davem@xxxxxxxxxxxxx>
In-reply-to: <20050211224354.GE31837@postel.suug.ch>
References: <420BF8CB.6080005@eurodev.net> <20050211032448.GD31837@postel.suug.ch> <420D243C.4090507@eurodev.net> <20050211224354.GE31837@postel.suug.ch>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5
Hi Thomas,

Thomas Graf wrote:

They are all the same thing. They all return 16 bytes which is the size of a netlink header. If you and someone else think that those are more readable, I'm ok with it, whatever. I just stole that piece of code as is from current checkings done in xfrm and rtnetlink.



That's absolutely true, my point is that while reading that line of code it reads like while the remaining length in the socket buffer is still greather or equal than the total message size until the next message with a payload of 0, then do something. It looks confusing at a first glance because you're not event interested in the payload just now. A choice of NLMSG_ALIGN(sizeof(*nlh)) or NLMSG_LENGTH(0) clearly points out that you're just checking for enough room regarding the netlink header in order to access it safely and do the real sanity check against the complete message length. Engross that thought a bit further, NLMSG_SPACE could change at some point assuming that it is only used in contexts where the total message size is in question.

If you think that's too much of nitpicking, then forget about it. It's
technicaly absolutely correct at the moment.



Looks fine you convince me. If this is finally pushed forward, let's replace that line. Thanks.


--
Pablo

<Prev in Thread] Current Thread [Next in Thread>