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

From: Linus Torvalds
Date: Fri May 17 2019 - 14:01:35 EST


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