linux-kernel - Re: [GIT PULL] treewide conversion to sizeof

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=wg8+eNK+SK1Ekqm0qNQHVM6e6YOdZx3yhsX6Ajo3gEupg@mail.gmail.com>
Date:   Thu, 26 Sep 2019 13:06:01 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@...el.com>,
        Joe Perches <joe@...ches.com>,
        Alexey Dobriyan <adobriyan@...il.com>
Subject: Re: [GIT PULL] treewide conversion to sizeof_member() for v5.4-rc1

On Thu, Sep 26, 2019 at 10:33 AM Kees Cook <keescook@...omium.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