Re: [PATCH] tools: gpio: fix debounce_period_us output of lsgpio

[Kent Gibson <warthog618@xxxxxxxxx>]

On Thu, May 04, 2023 at 11:22:17AM +0200, Milo Spadacini wrote:
> Fix wrong output that could occurs when more attributes are used and

      incorrect output        occur


>  GPIO_V2_LINE_ATTR_ID_DEBOUNCE is not the first one.

Have you actually seen this fault occur anywhere?  I would expect not,
as the debounce_period is the ONLY attr that is returned in practice by
existing kernels. The flags and values attributes are never returned in
the attrs of the gpio_v2_line_info - the flags are returned in their own
field, as they are always present, and the values are not returned in
the info at all.  Those types may be used in the request for multple
lines, but are not returned in the attrs for a single line.

> ---
>  tools/gpio/lsgpio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/gpio/lsgpio.c b/tools/gpio/lsgpio.c
> index c61d061247e1..52a0be45410c 100644
> --- a/tools/gpio/lsgpio.c
> +++ b/tools/gpio/lsgpio.c
> @@ -94,7 +94,7 @@ static void print_attributes(struct gpio_v2_line_info *info)
>         for (i = 0; i < info->num_attrs; i++) {
>                 if (info->attrs[i].id == GPIO_V2_LINE_ATTR_ID_DEBOUNCE)
>                         fprintf(stdout, ", debounce_period=%dusec",
> -                               info->attrs[0].debounce_period_us);
> +                               info->attrs[i].debounce_period_us);
>         }
>  }
> 

The fix itself is ok for debounce, so no issue with that.

But if there are multiple attributes, as you suggest in your comment,
then how about printing the rest of them rather than omitting them?
So perhaps replace this with an exhaustive decoder for all possible
attributes - probably implemented as a switch on the id and include a
"what the??" for the default case?
The flags and values types could be covered by the default, given we
don't expect them, but if so then add a comment to that effect.

Cheers,
Kent.