Re: [libgpiod v2][PATCH 2/6] API: rename gpiod_request_config_get_num_offsets to gpiod_request_config_get_num_lines to match line_request pattern

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Mar 15, 2022 at 12:23 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Tue, Mar 15, 2022 at 11:52:21AM +0100, Bartosz Golaszewski wrote:
> > On Fri, Mar 11, 2022 at 8:40 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > >
> > > Both gpiod_request_config and gpiod_line_request contain a number of
> > > lines, but the former has a get_num_offsets accessor, while the latter
> > > has get_num_lines.  Make them consistent by switching request_config to
> > > get_num_lines.
> > >
> > > Signed-off-by: Kent Gibson <warthog618@xxxxxxxxx>
> > > ---
> >
> > IMO this doesn't make much sense because we still have
> > gpiod_request_config_set_offsets(). So now you have set_offsets but
> > get_lines. At the time of preparing the request_config we're still
> > talking about offsets of lines to request, while once the request has
> > been made, we're talking about requested lines.
> >
>
> Oh FFS we are always talking about lines.  Whether you have requested
> them yet or not is irrelevant.  You WANT to request lines.
> If we had globally unique line names we wouldn't give a rats about the
> offset.
>
> And take another look - you have actually have get_offsets and
> get_num_lines functions.
>
> Offsets is just one of the attributes of the lines, and this approach
> still works if there is another fields of interest. e.g. values:
>
> int gpiod_line_request_set_values_subset(struct gpiod_line_request *request,
>                                          size_t num_lines,
>                                          const unsigned int *offsets,
>                                          const int *values);
>
> which you then complain about in patch 4 as I'm writing this.... <sigh>.
>
> You could equally argue that one should be num_values.
>
> While we are still preparing the configuration, we are preparing the
> config for LINES, not offsets.  Using num_lines is a reminder that you
> need to provide the offset for each line - the two are inextricably
> linked.  Using num_offsets could be taken to imply that
> gpiod_request_config_set_offsets() can be called multiple times to add
> offsets.
>
> > I would leave it as it is personally.
> >
>
> I know, I know :-|.
>
> Cheers,
> Kent.

I didn't know I would see the whole extend of Kent's wrath after that
comment. :)

Anyway let me try to defend myself before I wave the white flag and
surrender as usual.

We're setting VALUES so it's only normal to speak about NUMBER of VALUES.

It's like when you have an array of of X that is an attribute of Y,
that array still carries a number of X and not Y.

This is for patch 4 but this one has another problem. What if we
extend this structure to - besides offsets - accept string identifiers
for a request? Then we would want to use get_offsets and get_names (or
get_ids) and the corresponding get_num_offsets and get_num_names
accesors and in this case get_num_lines would become confusing.

Bart