public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	linux-gpio@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH v2] gpiolib: Fix line event handling in syscall compatible mode
Date: Tue, 15 Sep 2020 20:18:15 +0800	[thread overview]
Message-ID: <20200915121815.GA98308@sol> (raw)
In-Reply-To: <20200915092022.GR3956970@smile.fi.intel.com>

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.

  reply	other threads:[~2020-09-15 12:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-14 14:37 [PATCH v2] gpiolib: Fix line event handling in syscall compatible mode Andy Shevchenko
2020-09-14 14:55 ` Arnd Bergmann
2020-09-14 15:01   ` Andy Shevchenko
2020-09-14 23:05 ` Kent Gibson
2020-09-15  9:11   ` Andy Shevchenko
2020-09-15  9:20   ` Andy Shevchenko
2020-09-15 12:18     ` Kent Gibson [this message]
2020-09-15 12:56       ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200915121815.GA98308@sol \
    --to=warthog618@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=bgolaszewski@baylibre.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox