Re: [libgpiod][PATCH] treewide: rework struct gpiod_line_bulk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 23, 2020 at 4:16 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Fri, Oct 23, 2020 at 02:44:06PM +0200, Bartosz Golaszewski wrote:
> > On Fri, Oct 23, 2020 at 2:08 PM Andy Shevchenko
> > <andy.shevchenko@xxxxxxxxx> wrote:
> > >
> > > On Fri, Oct 23, 2020 at 3:06 PM Andy Shevchenko
> > > <andy.shevchenko@xxxxxxxxx> wrote:
> > > > On Fri, Oct 23, 2020 at 2:39 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> > > > > On Fri, Oct 23, 2020 at 12:24 PM Andy Shevchenko
> > > > > <andy.shevchenko@xxxxxxxxx> wrote:
> > >
> > > ...
> > >
> > > > > Nope because gcc will scream:
> > > > >
> > > > > error: flexible array member in union
> > > >
> > > > Ah, of course. Should be
> > > >   struct ... **lines;
> > >
> > > But it is not gonna work... we need an array here. or just one member
> > >
> > > struct *lines;
> > >
> > > bulk:
> > >   lines = malloc(num_lines * sizeof(lines));
> > >   xxx->lines = lines;
> > >
> > > single:
> > >   xxx->lines = line;
> >
> > The definition I used is clearer - it's explicit about using an array
> > with a single member by default and can be extended as needed when
> > allocating.
>
> According to [1] it makes harder to avoid sizeof() type of calculation
> mistakes for such struct.
>
> From my point of view extending something that has been already predefined
> is not cool. But it's up to you to decide.
>
> [1]: https://www.kernel.org/doc/html/v5.9-rc1/process/deprecated.html#zero-length-and-one-element-arrays
>

Your point is valid but we're not talking about a structure commonly
used across a vact code base that is the linux kernel. We're talking
about a structure that is private to a single .c file and its size is
calculated exactly once right next to where it's defined. Risk of
misusing it is miniscule IMO.

I think it's fine for what we need here: a struct that can be extended
when allocated dynamically but also which can be used on the stack for
the very precise use-case - holding a single line.

Bartosz



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux