Re: [libgpiod v2][PATCH 0/4] core: add gpiod_request_lines

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Thu, Mar 31, 2022 at 3:12 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> In my review of the CXX bindings I suggested a top-level version of
> Chip.request_lines(), and possibly a C API version as well, so here
> is the C version, plus a couple of semi-related tweaks I made along
> the way.
>
> The first patch adds the gpiod_request_lines().
> Patch 3 migrates the appropriate tools.
> Patch 4 minimizes the lifetimes of objects in the tools as I've
> previously seen confusion over how long lived objects need to be.
> Patch 2 is just a rename cos "inexistent" looks weird to me.
> Strictly speaking it is fine, but unless there is a problem with
> using "nonexistent" I would go with the latter.
>
> This series may be require my unsigned values patch.
>
> Cheers,
> Kent.
>
> Kent Gibson (4):
>   core: add gpiod_request_lines
>   tools: rename inexistent to nonexistent
>   tools: migrate to gpiod_request_lines
>   tools: minimize object lifetimes
>
>  include/gpiod.h            | 15 ++++++++
>  lib/line-request.c         | 17 +++++++++
>  tests/tests-line-request.c | 73 ++++++++++++++++++++++++++++++++++++++
>  tools/gpio-tools-test.bats |  4 +--
>  tools/gpiodetect.c         |  2 +-
>  tools/gpiofind.c           |  2 +-
>  tools/gpioget.c            | 25 +++++++------
>  tools/gpioinfo.c           |  4 +--
>  tools/gpiomon.c            | 16 ++++-----
>  tools/gpioset.c            | 18 +++++-----
>  tools/tools-common.c       | 50 ++++++++++----------------
>  tools/tools-common.h       |  5 +--
>  12 files changed, 164 insertions(+), 67 deletions(-)
>
> --
> 2.35.1
>

Ugh, I didn't respond under the C++ review in time before you spent
time on this. :/

I don't agree with this change. For C API I think the intention for v2
was to avoid having all kinds of high-level helpers and limit the
number of functions to only those that are necessary to fully leverage
the kernel uAPI and this one isn't necessary. I think we discussed it
multiple times and agreed that the C library needs to be minimal this
time.

For C++ and Python the issue is irrelevant because you can do:

auto request = gpiod::Chip("/dev/gpiochip0").request_lines(req_cfg, line_cfg);

or

request = gpiod.Chip("/dev/gpiochip0").request_lines(req_cfg, line_cfg)

respectively and achieve the same result while still using a one-liner.

Unless there's a *really* good reason to do this, it's a NAK from my side.

Bart