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] [day] [month] [year] [list]

Message-ID: <CAKFNMoksY6f5NtVwmn6K0K2QKTvdjq+s0FbdgLvHzS3YueKqYQ@mail.gmail.com>
Date: Thu, 9 Jan 2025 23:38:47 +0900
From: Ryusuke Konishi <konishi.ryusuke@...il.com>
To: "Brian G." <gissf1@...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

On Thu, Jan 9, 2025 at 10:44 PM Brian G. wrote:
>
> 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.

Hi Brian,

Thank you for your feedback!

Your help in pointing out the typos and the missing error code, as
well as suggestions for improving the "Return" section, are all very
helpful.
I'll update the patch set to reflect your comments.

Thanks,
Ryusuke Konishi