2021-05-20 13:08:55

by Lee Jones

[permalink] [raw]
Subject: [PATCH 1/1] fs: ext4: namei: trivial: Fix a couple of small whitespace issues

Cc: "Theodore Ts'o" <[email protected]>
Cc: Andreas Dilger <[email protected]>
Cc: Remy Card <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
fs/ext4/namei.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index afb9d05a99bae..7e780cf311c5a 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1899,7 +1899,7 @@ static struct ext4_dir_entry_2 *dx_pack_dirents(struct inode *dir, char *base,
* Returns pointer to de in block into which the new entry will be inserted.
*/
static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
- struct buffer_head **bh,struct dx_frame *frame,
+ struct buffer_head **bh, struct dx_frame *frame,
struct dx_hash_info *hinfo)
{
unsigned blocksize = dir->i_sb->s_blocksize;
@@ -2246,7 +2246,7 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
if (retval)
goto out_frames;

- de = do_split(handle,dir, &bh2, frame, &fname->hinfo);
+ de = do_split(handle, dir, &bh2, frame, &fname->hinfo);
if (IS_ERR(de)) {
retval = PTR_ERR(de);
goto out_frames;
--
2.31.1


2021-05-27 03:53:06

by Ritesh Harjani

[permalink] [raw]
Subject: Re: [PATCH 1/1] fs: ext4: namei: trivial: Fix a couple of small whitespace issues

On 21/05/20 01:55PM, Lee Jones wrote:

Hi Lee,

Thanks for your patch. I see we could a little better here.
There are several other checkpatch ERROR msgs in this file.
Care to fix all of those ERRORS within the same patch itself?

./scripts/checkpatch.pl -f fs/ext4/namei.c | sed -n '/ERROR/,/^$/p'

e.g. to list a few of them -
ERROR: do not use assignment in if condition
#1605: FILE: fs/ext4/namei.c:1605:
+ if ((bh = bh_use[ra_ptr++]) == NULL)

ERROR: space required after that ',' (ctx:VxV)
#1902: FILE: fs/ext4/namei.c:1902:
+ struct buffer_head **bh,struct dx_frame *frame,
^

ERROR: space required after that ',' (ctx:VxV)
#2249: FILE: fs/ext4/namei.c:2249:
+ de = do_split(handle,dir, &bh2, frame, &fname->hinfo);
^

ERROR: spaces required around that '=' (ctx:VxV)
#2288: FILE: fs/ext4/namei.c:2288:
+ int dx_fallback=0;

-ritesh

> Cc: "Theodore Ts'o" <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: Remy Card <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> fs/ext4/namei.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index afb9d05a99bae..7e780cf311c5a 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1899,7 +1899,7 @@ static struct ext4_dir_entry_2 *dx_pack_dirents(struct inode *dir, char *base,
> * Returns pointer to de in block into which the new entry will be inserted.
> */
> static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
> - struct buffer_head **bh,struct dx_frame *frame,
> + struct buffer_head **bh, struct dx_frame *frame,
> struct dx_hash_info *hinfo)
> {
> unsigned blocksize = dir->i_sb->s_blocksize;
> @@ -2246,7 +2246,7 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
> if (retval)
> goto out_frames;
>
> - de = do_split(handle,dir, &bh2, frame, &fname->hinfo);
> + de = do_split(handle, dir, &bh2, frame, &fname->hinfo);
> if (IS_ERR(de)) {
> retval = PTR_ERR(de);
> goto out_frames;
> --
> 2.31.1
>

2021-05-27 08:24:33

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/1] fs: ext4: namei: trivial: Fix a couple of small whitespace issues

On Thu, 27 May 2021, riteshh wrote:

> On 21/05/20 01:55PM, Lee Jones wrote:
>
> Hi Lee,
>
> Thanks for your patch. I see we could a little better here.
> There are several other checkpatch ERROR msgs in this file.
> Care to fix all of those ERRORS within the same patch itself?

I don't think it's a good idea to mix functionality within a single
patch. However, I would be happy to provide a follow-up solving these
issues for you.

> ./scripts/checkpatch.pl -f fs/ext4/namei.c | sed -n '/ERROR/,/^$/p'
>
> e.g. to list a few of them -
> ERROR: do not use assignment in if condition
> #1605: FILE: fs/ext4/namei.c:1605:
> + if ((bh = bh_use[ra_ptr++]) == NULL)
>
> ERROR: space required after that ',' (ctx:VxV)
> #1902: FILE: fs/ext4/namei.c:1902:
> + struct buffer_head **bh,struct dx_frame *frame,
> ^
>
> ERROR: space required after that ',' (ctx:VxV)
> #2249: FILE: fs/ext4/namei.c:2249:
> + de = do_split(handle,dir, &bh2, frame, &fname->hinfo);
> ^
>
> ERROR: spaces required around that '=' (ctx:VxV)
> #2288: FILE: fs/ext4/namei.c:2288:
> + int dx_fallback=0;
>
> -ritesh
>
> > Cc: "Theodore Ts'o" <[email protected]>
> > Cc: Andreas Dilger <[email protected]>
> > Cc: Remy Card <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > fs/ext4/namei.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> > index afb9d05a99bae..7e780cf311c5a 100644
> > --- a/fs/ext4/namei.c
> > +++ b/fs/ext4/namei.c
> > @@ -1899,7 +1899,7 @@ static struct ext4_dir_entry_2 *dx_pack_dirents(struct inode *dir, char *base,
> > * Returns pointer to de in block into which the new entry will be inserted.
> > */
> > static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
> > - struct buffer_head **bh,struct dx_frame *frame,
> > + struct buffer_head **bh, struct dx_frame *frame,
> > struct dx_hash_info *hinfo)
> > {
> > unsigned blocksize = dir->i_sb->s_blocksize;
> > @@ -2246,7 +2246,7 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
> > if (retval)
> > goto out_frames;
> >
> > - de = do_split(handle,dir, &bh2, frame, &fname->hinfo);
> > + de = do_split(handle, dir, &bh2, frame, &fname->hinfo);
> > if (IS_ERR(de)) {
> > retval = PTR_ERR(de);
> > goto out_frames;
> >

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

2021-07-22 11:36:10

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/1] fs: ext4: namei: trivial: Fix a couple of small whitespace issues

On Thu, 20 May 2021, Lee Jones wrote:

> Cc: "Theodore Ts'o" <[email protected]>
> Cc: Andreas Dilger <[email protected]>
> Cc: Remy Card <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> fs/ext4/namei.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Any news on this please?

Would you like me to submit a [RESEND]?

> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index afb9d05a99bae..7e780cf311c5a 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1899,7 +1899,7 @@ static struct ext4_dir_entry_2 *dx_pack_dirents(struct inode *dir, char *base,
> * Returns pointer to de in block into which the new entry will be inserted.
> */
> static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
> - struct buffer_head **bh,struct dx_frame *frame,
> + struct buffer_head **bh, struct dx_frame *frame,
> struct dx_hash_info *hinfo)
> {
> unsigned blocksize = dir->i_sb->s_blocksize;
> @@ -2246,7 +2246,7 @@ static int make_indexed_dir(handle_t *handle, struct ext4_filename *fname,
> if (retval)
> goto out_frames;
>
> - de = do_split(handle,dir, &bh2, frame, &fname->hinfo);
> + de = do_split(handle, dir, &bh2, frame, &fname->hinfo);
> if (IS_ERR(de)) {
> retval = PTR_ERR(de);
> goto out_frames;

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

2021-07-22 22:17:56

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH 1/1] fs: ext4: namei: trivial: Fix a couple of small whitespace issues

On Thu, Jul 22, 2021 at 12:35:43PM +0100, Lee Jones wrote:
> On Thu, 20 May 2021, Lee Jones wrote:
>
> > Cc: "Theodore Ts'o" <[email protected]>
> > Cc: Andreas Dilger <[email protected]>
> > Cc: Remy Card <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > fs/ext4/namei.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Any news on this please?
>
> Would you like me to submit a [RESEND]?

Please don't send "checkpatch.pl --file" patches for the ext4 file
system; if you must, please focus on files in the drivers directory,
where they are more welcome. If developers are making changes to a
file, fixing some checkpatch.pl whines is fine, but white-sapace only
changes just obfuscates "git blame" code archology, and so the costs
far outwieghs the costs. "Fix" is also not the right verb to use.
For more information please see [1].

[1] https://gist.github.com/17twenty/8154928

If you are looking for subtantive ways of contributing to the ext4
file system, feel free to look at various syzbot warnings[2] and try
to figure out what is going on there.

[2] https://syzkaller.appspot.com/upstream

(In some cases, the syzbot complaint has already been fixed, and it's
just a matter of letting syzbot knoww that it has since been fixed by
a particular commit. See [3] for more details.)

[3] https://github.com/google/syzkaller/blob/master/docs/syzbot.md

Cheers,

- Ted

2021-07-23 07:26:27

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH 1/1] fs: ext4: namei: trivial: Fix a couple of small whitespace issues

On Thu, 22 Jul 2021, Theodore Ts'o wrote:

> On Thu, Jul 22, 2021 at 12:35:43PM +0100, Lee Jones wrote:
> > On Thu, 20 May 2021, Lee Jones wrote:
> >
> > > Cc: "Theodore Ts'o" <[email protected]>
> > > Cc: Andreas Dilger <[email protected]>
> > > Cc: Remy Card <[email protected]>
> > > Cc: "David S. Miller" <[email protected]>
> > > Cc: [email protected]
> > > Signed-off-by: Lee Jones <[email protected]>
> > > ---
> > > fs/ext4/namei.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Any news on this please?
> >
> > Would you like me to submit a [RESEND]?
>
> Please don't send "checkpatch.pl --file" patches for the ext4 file
> system; if you must, please focus on files in the drivers directory,
> where they are more welcome. If developers are making changes to a
> file, fixing some checkpatch.pl whines is fine, but white-sapace only
> changes just obfuscates "git blame" code archology, and so the costs
> far outwieghs the costs. "Fix" is also not the right verb to use.
> For more information please see [1].
>
> [1] https://gist.github.com/17twenty/8154928
>
> If you are looking for subtantive ways of contributing to the ext4
> file system, feel free to look at various syzbot warnings[2] and try
> to figure out what is going on there.
>
> [2] https://syzkaller.appspot.com/upstream
>
> (In some cases, the syzbot complaint has already been fixed, and it's
> just a matter of letting syzbot knoww that it has since been fixed by
> a particular commit. See [3] for more details.)
>
> [3] https://github.com/google/syzkaller/blob/master/docs/syzbot.md

This patch doesn't have anything to do with checkpatch.

Daniel, who was making more 'substantive' changes to ext4, fixed these
whitespace issues in his first submission [0], but were dropped from
the second revision for some reason and thus didn't make it into
Mainline [1].

This is an attempt to realign his development repo (Android) with
Mainline. I had the choice of either backporting the issues back into
the Android kernel or upstreaming the original whitespace cleanup.

It made more sense to me to draft a patch making one codebase better
than another one worse.

[0] https://lore.kernel.org/linux-fsdevel/[email protected]/
[1] 471fbbea7ff70 ext4: handle casefolding with encryption

--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog