Re: [PATCH v2] gpiolib: Fix line event handling in syscall compatible mode

Kent Gibson
2020-09-15T12:20:48Z

On Tue, Sep 15, 2020 at 12:20:22PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 15, 2020 at 07:05:26AM +0800, Kent Gibson wrote:
> > On Mon, Sep 14, 2020 at 05:37:43PM +0300, Andy Shevchenko wrote:
>
> ...
>
> > It can return size_t now.
>
> > > ssize_t bytes_read = 0;
> > > + ssize_t ge_size;
> >
> > Similarly here.
>
> I deliberately left the ssize_t type to be consistent with the returned type of
> the function and bytes_read. If you insist on the type change I will do, though
> I personally like my approach.
>

Bart prefers to use unsigned ints where variables are never negative,
and lineevent_get_size() never returns negative so should be size_t.
And it feels like a sizeof() to me so should return a size_t.

By the same logic bytes_read is never negative so it should be size_t as
well. It seems reasonable to assume that bytes_read will always be less
than SSIZE_MAX so any cast to ssize_t for the return would be harmless.
Though changing that would probably mean a separate patch?

> Thanks for your review. Before I'm going on it, can you confirm that these are
> the only issues with the patch and after addressing them you will be okay with
> the patch?

I have suggested renaming ge_size to event_size, but that is just personal
preference. You have more than enough documentation describing the issue
where it is assigned, so I'm fine with that.

These are just my suggestions. Feel free to ignore them.

Cheers,
Kent.