On Thu, 3 Aug 2023 14:29:28 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:On Thu, Aug 03, 2023 at 12:09:16PM +1000, David Gibson wrote:Right, I was about to write you that... or rather, NLMSG_LENGTH() is the (presumably) lesser-aligned length. Also, if you check pretty much any example in iproute2, nlmsg_len is always set like that, using NLMSG_LENGTH() on the payload.On Thu, Aug 03, 2023 at 12:47:29AM +0200, Stefano Brivio wrote:[snip]Duh, sorry, I realized I had this backwards. NLSMSG_LENGTH() is the non-aligned length, sizeof() may include alignment. I'll rework based on that understanding.Oof... so. On the one hand, I see the issue; if these are different, I'm not sure what the effect will be. On the other hand, if we use NLMSG_LENGTH and it *is* longer than the structure size, we'll be saying that this message is longer than the datagram containing it. I'm not sure what the effect of that will be either.-void nl_link(int ns, unsigned int ifi, void *mac, int up, int mtu) +void nl_link_get_mac(int ns, unsigned int ifi, void *mac) { - int change = !MAC_IS_ZERO(mac) || up || mtu; struct req_t { struct nlmsghdr nlh; struct ifinfomsg ifm; - struct rtattr rta; - union { - unsigned char mac[ETH_ALEN]; - struct { - unsigned int mtu; - } mtu; - } set; } req = { - .nlh.nlmsg_type = change ? RTM_NEWLINK : RTM_GETLINK, - .nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)), - .nlh.nlmsg_flags = NLM_F_REQUEST | (change ? NLM_F_ACK : 0), + .nlh.nlmsg_type = RTM_GETLINK, + .nlh.nlmsg_len = sizeof(req),I don't think there's a practical issue with this, but there were two reasons why I used NLMSG_LENGTH(sizeof(struct ifinfomsg)) instead: - NLMSG_LENGTH() aligns to 4 bytes, not to whatever architecture-dependent alignment we might have: the message might actually be smaller> Not really sure what to do about this. > > > - I see that this works with gcc and clang, but, strictly > > speaking, is the size of the struct known "before" > > (sequence-point-wise) we're done initialising it? I have a very vague > > memory of this not working with gcc 2.9 or suchlike -- which is not a > > problem, as long as our new friend C11 actually supports this (but > > I'm not entirely sure). > > I'm pretty sure it's ok, regardless of C11 state. It's not really a > question of sequence points: those are about the ordering of run time > operations. Even though the structure is being defined inline, > determining it's size and layout will still happen at compile time, > whereas the initialization is obviously a runtime event.Ah, sorry, yes, of course. Still I remember that failing spectacularly in a distant past. But you just checked with gcc 4-ish I guess, so I guess it would have been fine anyway. -- Stefano