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

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Fri, Oct 23, 2020 at 12:31 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
> From: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
>
> Current implementation of struct gpiod_line_bulk uses stack memory
> excessively. The structure is big: it's an array 64 pointers + 4 bytes
> size. That amounts to 260 bytes on 32-bit and 516 bytes on 64-bit
> architectures respectively. It's also used everywhere as all functions
> dealing with single lines eventually end up calling bulk counterparts.
>
> The rework addresses it by making the bulk structure opaque and
> providing appropriate interfaces for library users while retaining a way
> for internal users to allocate single line bulks on the stack.
>
> The macro-based loop has been removed. In its place we provide a function
> iterating over all lines held by a bulk and calling the provided callback
> function for each line as well as a new line bulk iterator which works
> similarily to chip and line iterators.

similarly

> Since bulk operations can now fail, a bunch of test-cases has been added
> to cover the relevant code.
>
> While at it: using the word offset both when referring to line's HW
> offset in a chip as well as the offset in a bulk leads to confusion.
> This patch renames the bulk offset to index.

...

> +struct gpiod_line_bulk {
> +       struct gpiod_chip *owner;
> +       unsigned int num_lines;
> +       unsigned int max_lines;
> +       struct gpiod_line *lines[1];

Why not '[]' as we do in the kernel?

> +};

> +#define BULK_SINGLE_LINE_INIT(line) \
> +               { gpiod_line_get_chip(line), 1, 1, { (line) } }

Hmm... Perhaps union can help here?

union {
  struct *lines[];
  struct *line;
}

num_lines == 1 exactly defines this.

-- 
With Best Regards,
Andy Shevchenko