linux-kernel - Re: [PATCH] pipe_fs_i.h: add pipe_buf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f185463b-4781-4e63-894b-c3592e3c0852@infradead.org>
Date:   Tue, 19 Sep 2023 14:40:55 -0700
From:   Randy Dunlap <rdunlap@...radead.org>
To:     Max Kellermann <max.kellermann@...os.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Hugh Dickins <hughd@...gle.com>
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [PATCH] pipe_fs_i.h: add pipe_buf_init()

Hi--

On 9/19/23 01:07, Max Kellermann wrote:
> Adds one central function which shall be used to initialize a newly
> allocated struct pipe_buffer.  This shall make the pipe code more
> robust for the next time the pipe_buffer struct gets modified, to
> avoid leaving new members uninitialized.  Instead, adding new members
> should also add a new pipe_buf_init() parameter, which causes
> compile-time errors in call sites that were not adapted.
> 
> This commit doesn't refactor fs/fuse/dev.c because this code looks
> obscure to me; it initializes pipe_buffers incrementally through a
> variety of functions, too complicated for me to understand.
> 
> Signed-off-by: Max Kellermann <max.kellermann@...os.com>
> ---
>  fs/pipe.c                 |  9 +++------
>  fs/splice.c               |  9 ++++-----
>  include/linux/pipe_fs_i.h | 20 ++++++++++++++++++++
>  kernel/watch_queue.c      |  8 +++-----
>  mm/filemap.c              |  8 ++------
>  mm/shmem.c                |  9 +++------
>  6 files changed, 35 insertions(+), 28 deletions(-)
> 

> diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
> index 608a9eb86bff..2ef2bb218641 100644
> --- a/include/linux/pipe_fs_i.h
> +++ b/include/linux/pipe_fs_i.h
> @@ -176,6 +176,26 @@ static inline struct pipe_buffer *pipe_head_buf(const struct pipe_inode_info *pi
>  	return pipe_buf(pipe, pipe->head);
>  }
>  
> +/**
> + * Initialize a struct pipe_buffer.
> + */

That's not a kernel-doc comment so don't begin it with /**.
Just use /* instead.
Thanks.

> +static inline void pipe_buf_init(struct pipe_buffer *buf,
> +				 struct page *page,
> +				 unsigned int offset, unsigned int len,
> +				 const struct pipe_buf_operations *ops,
> +				 unsigned int flags)
> +{
> +	buf->page = page;
> +	buf->offset = offset;
> +	buf->len = len;
> +	buf->ops = ops;
> +	buf->flags = flags;
> +
> +	/* not initializing the "private" member because it is only
> +	   used by pipe_buf_operations which inject it via struct
> +	   partial_page / struct splice_pipe_desc */
> +}
> +
>  /**
>   * pipe_buf_get - get a reference to a pipe_buffer
>   * @pipe:	the pipe that the buffer belongs to


-- 
~Randy