All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Tejun Heo <tj@kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Amir Goldstein <amir73il@gmail.com>, Jan Kara <jack@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Linux FS-devel Mailing List <linux-fsdevel@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>
Subject: Re: [PATCH] seq_file: fix clang warning for NULL pointer arithmetic
Date: Tue, 27 Oct 2020 15:51:22 +0100	[thread overview]
Message-ID: <CAK8P3a0irGzw8YDdV9HoaaiPOfgzWQ6hxgbC6_dx=4E8vGKXXA@mail.gmail.com> (raw)
In-Reply-To: <20201027104450.GA8864@infradead.org>

On Tue, Oct 27, 2020 at 11:45 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> > index f277d023ebcd..b55e6ef4d677 100644
> > --- a/fs/kernfs/file.c
> > +++ b/fs/kernfs/file.c
> > @@ -124,7 +124,7 @@ static void *kernfs_seq_start(struct seq_file *sf, loff_t *ppos)
> >                * The same behavior and code as single_open().  Returns
> >                * !NULL if pos is at the beginning; otherwise, NULL.
> >                */
> > -             return NULL + !*ppos;
> > +             return (void *)(uintptr_t)!*ppos;
>
> Yikes.  This is just horrible, why bnot the completely obvious:
>
>         if (ops->seq_start) {
>                 ...
>                 return next;
>         }
>
>         if (*ppos)
>                 return NULL;
>         return ppos; /* random cookie */

I was trying to not change the behavior, but I guess we can do better
than either the original version mine. Not sure I'd call your version
'obvious' either though, at least it was immediately clear to me that
returning an unrelated pointer here is the right thing to do (it works,
since it is guaranteed to be neither NULL nor an error pointer
and it is never dereferenced, but it's still odd).

I'd rather define something like

#define SEQ_OPEN_SINGLE (void *)1ul

and return that here. I'll send a patch doing that, let me know what
you think.

     Arnd

      reply	other threads:[~2020-10-27 17:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-26 21:52 [PATCH] seq_file: fix clang warning for NULL pointer arithmetic Arnd Bergmann
2020-10-27  1:49 ` Nathan Chancellor
2020-10-27 10:44 ` Christoph Hellwig
2020-10-27 14:51   ` Arnd Bergmann [this message]

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='CAK8P3a0irGzw8YDdV9HoaaiPOfgzWQ6hxgbC6_dx=4E8vGKXXA@mail.gmail.com' \
    --to=arnd@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=amir73il@gmail.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=natechancellor@gmail.com \
    --cc=ndesaulniers@google.com \
    --cc=tj@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.