linux-kernel - Re: [PATCH 1/6] nilfs2: correct return value kernel-doc descriptions for ioctl functions

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: <CAAq45aNh1qV8P6XgDhKeNstT=PvcPUaCXsAF-f9rvmzznsZL5A@mail.gmail.com>
Date: Thu, 9 Jan 2025 07:44:38 -0600
From: "Brian G." <gissf1@...il.com>
To: Ryusuke Konishi <konishi.ryusuke@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-nilfs@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] nilfs2: correct return value kernel-doc descriptions
 for ioctl functions

Hello Ryusuke,

I'm somewhat new to public linux kernel contributions, but I have been
following the NILFS2 mailing list for a while now.  I know you have
been putting in incredible effort into supporting NILFS2, and I
appreciate the time and dedication you have provided to the community
on this.

I think the new description comments look much cleaner and easier to
read, but while reviewing your changeset, I noticed a few small
concerns.  I will describe them inline below.

On Wed, Jan 8, 2025 at 9:28 PM Ryusuke Konishi
<konishi.ryusuke@...il.com> wrote:
> ---
>  fs/nilfs2/ioctl.c | 220 ++++++++++++++++++----------------------------
>  1 file changed, 84 insertions(+), 136 deletions(-)
>
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index fa77f78df681..f7bcc95847bb 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -457,7 +439,8 @@ nilfs_ioctl_do_get_vinfo(struct the_nilfs *nilfs, __u64 *posp, int flags,
> - * Return value: count of nilfs_bdescs structures in output buffer.
> + * Return: Count of nilfs_bdescs structures in output buffer on succes, or
Typo: "succes" should be "success"

> @@ -494,19 +477,14 @@ nilfs_ioctl_do_get_bdescs(struct the_nilfs *nilfs, __u64 *posp, int flags,
> - * %-EFAULT - Failure during getting disk block descriptors.
> + * * %-EFAULT  - Failure during getting dick block descriptors.
Typo: "dick" should be "disk"

> @@ -1202,18 +1156,12 @@ static int nilfs_ioctl_get_info(struct inode *inode, struct file *filp,
> - * %-ENOMEM - Insufficient amount of memory available.
The error value -ENOMEM can still be returned by the new code, but it
is not included in the new error list

Also, throughout the changeset, in multiple files, I saw lines like:
> + * Return: 0 on success, or the following error code on failure.

These lines should probably instead be something like:
> + * Return: 0 on success, or one of the following negative error codes on failure:
Noting the specific differences of:
- "one of" prefix before "the following" when there are more than 1
option in the list
- be sure to include the word "negative" (some cases did, others did not)
- using the plural form "codes" instead of singular "code" when there
are more than 1 option in the list
- trailing ":" to indicate that a list follows

I hope this email was helpful, and thanks again for all you do!

- Brian G.