On Thu, Sep 26, 2019 at 10:33 AM Kees Cook <keesc...@chromium.org> wrote: > > Please pull this mostly mechanical treewide conversion to the single and > more accurately named sizeof_member() macro for the end of v5.4-rc1. This > replaces 3 macros of the same behavior (FIELD_SIZEOF(), SIZEOF_FIELD(), > and sizeof_field()). The last patch in the series has a script in the > commit log to do the conversion, if you want to compare the results > (they remained identical today when I checked).
Honestly, I'm not sure why "sizeof_field()" wasn't just picked when we already had it. Making a new macro for the exact same thing seems somewhat questionable. Yes, yes, the C standard calls them "members". Except when it doesn't, and they are members of a bit type, and it calls them bit-fields. And 'field' is a fairly common use outside of the standard - including among compilers, but also in the kernel. Look at a lot of the tracing code in the kernel, for example., So we did have that existing macro, with a reasonable name, and it did have some use, and it's not hard to understand. I also note that we don't actually use the word "member" much in the kernel: $ git grep -iw member | wc 3302 25603 280767 $ git grep -iw field | wc 26082 213732 2253806 that's an almost order-of-magnitude vote in favor of "field" as a word, fwiw. HOWEVER. Another issue is that most of the patch is for the use by far is the FIELD_SIZEOF macro. And the issue I have there is that while I think that could have a better name - the upper-casing doesn't match offsetof or sizeof or any of the existing struct/union member things - the _big_ user is networking code. And I don't see anybody having actually talked to the networking people whose code-base this mostly touches. Something like 80% of the patch is to that subsystem (and it would have been even higher, if it wasn't for the forced "member" renaming), I'd really like to see an Ack or similar. Maybe one was given during the discussions, but it's not visible in the pull request or the git tree. So (a) why didn't this use the already existing and well-named macro that nobody really had issues with? (b) I see no sign of the networking people having been asked about their preferences. Hmm> Linus