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

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Sep 15, 2020 at 08:18:15PM +0800, Kent Gibson wrote:
> 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.

Thanks for review!

I will send v3 soon, but I will leave ssize_t by the reasons I mentioned above.

-- 
With Best Regards,
Andy Shevchenko