LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tracing: silence GCC 9 array bounds warning
Date: Fri, 17 May 2019 10:59:07 -0700	[thread overview]
Message-ID: <CAHk-=wiNkOU-Ng+9_+tj4-AqJ4Q9JQpVbR4QVVAWLY68yQ62Gw@mail.gmail.com> (raw)
In-Reply-To: <20190517092502.GA22779@gmail.com>

On Fri, May 17, 2019 at 2:25 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.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

  parent reply	other threads:[~2019-05-17 17:59 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-17  9:25 Miguel Ojeda
2019-05-17 16:47 ` Steven Rostedt
2019-05-17 18:45   ` Miguel Ojeda
2019-05-17 17:59 ` Linus Torvalds [this message]
2019-05-17 19:09   ` Miguel Ojeda
2019-05-19 21:35     ` Steven Rostedt
2019-05-17 20:54 ` Miguel Ojeda
2019-05-23 12:45 Miguel Ojeda
2019-05-24  2:12 ` Steven Rostedt
2019-05-24  4:05   ` Miguel Ojeda
2019-05-24 11:52     ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wiNkOU-Ng+9_+tj4-AqJ4Q9JQpVbR4QVVAWLY68yQ62Gw@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --subject='Re: [PATCH] tracing: silence GCC 9 array bounds warning' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).