public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Stephen Zhang <starzhangzsd@gmail.com>
Cc: djwong@kernel.org, dchinner@redhat.com, chandan.babu@oracle.com,
	zhangshida@kylinos.cn, linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfs: fix up the comment in xfs_dir2_isleaf
Date: Mon, 12 Sep 2022 08:20:24 +1000	[thread overview]
Message-ID: <20220911222024.GY3600936@dread.disaster.area> (raw)
In-Reply-To: <20220911033137.4010427-1-zhangshida@kylinos.cn>

On Sun, Sep 11, 2022 at 11:31:37AM +0800, Stephen Zhang wrote:
> xfs_dir2_isleaf is used to see if the directory is a single-leaf
> form directory, as commented right above the function.
> 
> Signed-off-by: Shida Zhang <zhangshida@kylinos.cn>
> ---
>  fs/xfs/libxfs/xfs_dir2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index 76eedc2756b3..1485f53fecf4 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -632,7 +632,7 @@ xfs_dir2_isblock(
>  int
>  xfs_dir2_isleaf(
>  	struct xfs_da_args	*args,
> -	int			*vp)	/* out: 1 is block, 0 is not block */
> +	int			*vp)	/* out: 1 is leaf, 0 is not leaf */
>  {
>  	xfs_fileoff_t		last;	/* last file offset */
>  	int			rval;

If you are going to touch this code to fix a broken comment, then
please clean it up properly.

The "*vp" parameter should be a "bool *isleaf", in which case the
return value is obvious and the comment can be removed. Then the
logic in the function can be cleaned up to be obvious instead of
relying on easy to mistake conditional logic in assignemnts...

IOWs, don't fix the comments - fix the code to be self documenting
to remove the need for comments...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2022-09-11 22:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-11  3:31 [PATCH] xfs: fix up the comment in xfs_dir2_isleaf Stephen Zhang
2022-09-11 22:20 ` Dave Chinner [this message]
2022-09-12  3:14   ` Stephen Zhang
2022-09-12  5:00     ` Dave Chinner

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=20220911222024.GY3600936@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=chandan.babu@oracle.com \
    --cc=dchinner@redhat.com \
    --cc=djwong@kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=starzhangzsd@gmail.com \
    --cc=zhangshida@kylinos.cn \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox