Re: [libgpiod][PATCH v2 2/4] tools: use ppoll() where higher timeout resolution makes sense

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Wed, Apr 17, 2024 at 9:23 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Tue, Apr 16, 2024 at 11:52:20PM +0200, Bartosz Golaszewski wrote:
> >
> > -             ret = poll(pollfds, resolver->num_chips, cfg.idle_timeout);
> > +             if (cfg.idle_timeout > 0) {
> > +                     idle_timeout.tv_sec = cfg.idle_timeout / 1000000;
> > +                     idle_timeout.tv_nsec =
> > +                                     (cfg.idle_timeout % 1000000) * 1000;
> > +             }
> > +
> > +             ret = ppoll(pollfds, resolver->num_chips,
> > +                         cfg.idle_timeout > 0 ? &idle_timeout : NULL, NULL);
> >               if (ret < 0)
> >                       die_perror("error polling for events");
> >
>
> One minor nit - I would introduce a timespec pointer initialised to NULL
> and set to point to idle_timeout within the if rather than repeat the
> cfg.idle_timeout > 0 test.
>

Actually we can avoid it by doing it once before we enter the for(;;)
loop. It's passed by constant pointer to ppoll() anyway and having the
struct AND pointer to it initialized to NULL sounds more complex than
it needs to be. I'll do it in v2.

> But that is just personal preference, so either way,
>
> Reviewed-by: Kent Gibson <warthog618@xxxxxxxxx>
>

Thanks, I'll keep the tag if you don't mind the above solution?

Bart