Message-ID: <20200604182123.GD10051@embeddedor>
Date:   Thu, 4 Jun 2020 13:21:23 -0500
From:   "Gustavo A. R. Silva" <gustavoars@...nel.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Subject: Re: [PATCH] docs: deprecated.rst: Add note to the use of
 struct_size() helper

On Thu, Jun 04, 2020 at 10:49:19AM -0700, Kees Cook wrote:
> On Thu, Jun 04, 2020 at 12:21:38PM -0500, Gustavo A. R. Silva wrote:
> > Add a note to educate people about the proper use of struct_size() when
> > the trailing array in the enclosing structure is a one-element array.
> > 
> > Zero-length and one-element arrays will soon be removed from the kernel,
> > but in the meantime, it's worth letting people know how to correctly
> > use struct_size() together with such constructs.
> > 
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
> > ---
> > 
> > Kees,
> >  
> > This is not substitute for the patch I'll write about flexible-arrays
> > and the deprecation of zero-lenght and one-element arrays.
> 
> Hm, hm. I think I'd rather just get the 0/1-array docs written, since
> that will mean this paragraph isn't needed at all. (Or rather, it can be

Yeah. My reasoning for is that it will take a while --at least one 
development cycle more-- to completely get rid of all the 0/1-arrays.
Also, this was motivated by the following comments from Christian
König:

"May I suggest that we add a section how to correctly do this to
Documentation/process/coding-style.rst or similar document?

I've seen a bunch of different approaches and some even doesn't work
with some gcc versions and result in a broken binary."[1]

> modified to say "if you're using struct_size() on a 1-array, stop using
> a 1-array, see [link]". If someone is going to switch around their code,
> they need to switch to flex at the same time, IMO.
> 

I agree with this. I can add the comments in quotes you suggest to this
patch.

But I think we can add this note while I continue working on the flexible-array
conversions. Once that work is complete, I can go back and update the
documentation. :)

What do you think?

Thanks
--
Gustavo

[1] https://lore.kernel.org/lkml/1065d63e-7959-e4b4-af4e-70607ba92296@amd.com/

> > 
> >  Documentation/process/deprecated.rst | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst
> > index 652e2aa02a66c..0b7b37718bf96 100644
> > --- a/Documentation/process/deprecated.rst
> > +++ b/Documentation/process/deprecated.rst
> > @@ -85,6 +85,17 @@ Instead, use the helper::
> >  
> >  	header = kzalloc(struct_size(header, item, count), GFP_KERNEL);
> >  
> > +NOTE: You might want to use the following form in case the trailing array
> > +is a one-element array, as unlike zero-length arrays and flexible-array
> > +members, `one-element arrays occupy at least as much space as a single
> > +object of the type <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_,
> > +hence they contribute to the size of the enclosing structure::
> > +
> > +	header = kzalloc(struct_size(header, item, count - 1), GFP_KERNEL);
> > +
> > +It's also worth noting that one-element arrays --together with zero-length
> > +arrays-- will soon be completely removed from the codebase and deprecated.
> > +
> >  See array_size(), array3_size(), and struct_size(),
> >  for more details as well as the related check_add_overflow() and
> >  check_mul_overflow() family of functions.
> > -- 
> > 2.27.0
> > 
> 
> -- 
> Kees Cook

Powered by blists - more mailing lists