linux-kernel - Re: [PATCH] tracing: silence GCC 9 array bounds warning

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAHk-=wiNkOU-Ng+9_+tj4-AqJ4Q9JQpVbR4QVVAWLY68yQ62Gw@mail.gmail.com>
Date:   Fri, 17 May 2019 10:59:07 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tracing: silence GCC 9 array bounds warning

On Fri, May 17, 2019 at 2:25 AM Miguel Ojeda
<miguel.ojeda.sandonis@...il.com> wrote:
>
> +       memset((char *)(iter) + offsetof(struct trace_iterator, seq), 0,
> +              sizeof(struct trace_iterator) -
> +              offsetof(struct trace_iterator, seq));

Honestly, the above is nasty.

Whenever you have to split an expression or statement over several
lines, you should ask yourself why it's so complicated.

This can be trivially rewritten as

        const unsigned int offset = offsetof(struct trace_iterator, seq);
        memset(offset + (void *)iter, 0, sizeof(*iter) - offset);

and now we have two much simpler expressions that are each much easier
to read and don't need multiple lines.

In particular using that "offset" variable means that you can
trivially see that "oh, we're starting the memset at that offset, and
we're using the 'sizeof() - offset' thing to limit the size". It's a
lot clearer to use the same trivial 'offset' thing in both the offset
and the size, than to have a more complex expression that it
duplicated twice and is not at all as visually obvious that it's the
exact same thing simply because it's bigger and more complex.

Also, the while 'offset' is a variable, any compiler will immediately
see that it's a constant value, so it's not like this will affect the
generated code at all. Unless you compile with something crazy like
'-O0', which is not a supported configuration exactly because we
expect compilers to not be terminally stupid.

So you get simpler and clearer source, with no downsides.

               Linus