On Tue, Oct 27, 2020 at 11:18:24PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> Clang points out that adding something to NULL is notallowed
not allowed
> in standard C:
>
> fs/kernfs/file.c:127:15: warning: performing pointer arithmetic on a
> null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> return NULL + !*ppos;
> ~~~~ ^
> fs/seq_file.c:529:14: warning: performing pointer arithmetic on a
> null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> return NULL + (*pos == 0);
>
> Rephrase the code to be extra explicit about the valid, giving
valid what?
> them named SEQ_OPEN_EOF and SEQ_OPEN_SINGLE definitions.
> The instance in kernfs was copied from single_start, so fix both
> at once.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Fixes: c2b19daf6760 ("sysfs, kernfs: prepare read path for kernfs")
> Signed-off-by: Arnd Bergmann <[email protected]>
Aside from the minor nits in the commit message, this should keep the
same behavior while eliminating the warning and making it a little
easier to understand.
Reviewed-by: Nathan Chancellor <[email protected]>
> ---
> v2: add the named macros after Christoph Hellwig pointed out
> that my original logic was too ugly.
> Suggestions for better names welcome
>
> v3: don't overload the NULL return, avoid ?: operator
> ---
> fs/kernfs/file.c | 9 ++++++---
> fs/seq_file.c | 5 ++++-
> include/linux/seq_file.h | 2 ++
> 3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
> index f277d023ebcd..5a5adb03c6df 100644
> --- a/fs/kernfs/file.c
> +++ b/fs/kernfs/file.c
> @@ -121,10 +121,13 @@ static void *kernfs_seq_start(struct seq_file *sf,
> loff_t *ppos)
> return next;
> } else {
> /*
> - * The same behavior and code as single_open(). Returns
> - * !NULL if pos is at the beginning; otherwise, NULL.
> + * The same behavior and code as single_open(). Continues
> + * if pos is at the beginning; otherwise, NULL.
> */
> - return NULL + !*ppos;
> + if (*ppos)
> + return NULL;
> +
> + return SEQ_OPEN_SINGLE;
> }
> }
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index 31219c1db17d..6b467d769501 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -526,7 +526,10 @@ EXPORT_SYMBOL(seq_dentry);
>
> static void *single_start(struct seq_file *p, loff_t *pos)
> {
> - return NULL + (*pos == 0);
> + if (*pos)
> + return NULL;
> +
> + return SEQ_OPEN_SINGLE;
> }
>
> static void *single_next(struct seq_file *p, void *v, loff_t *pos)
> diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
> index 813614d4b71f..eb344448d4da 100644
> --- a/include/linux/seq_file.h
> +++ b/include/linux/seq_file.h
> @@ -37,6 +37,8 @@ struct seq_operations {
>
> #define SEQ_SKIP 1
>
> +#define SEQ_OPEN_SINGLE (void *)1
> +
> /**
> * seq_has_overflowed - check if the buffer has overflowed
> * @m: the seq_file handle
> --
> 2.27.0
>
