[PATCH 0/2] Submitting my first patch series (Checkpatch fixes)

[PATCH 0/2] Submitting my first patch series (Checkpatch fixes)

[Caitlyn]

Hello!

This patch series cleans up some checkpatch fixes in erofs. The patches
include balancing conditional braces and fixing some indentation. No testing
done, all patches build and checkpath cleanly.

Caitlyn (2):
  staging/erofs/xattr.h: Fixed misaligned function arguments.
  staging/erofs: Balanced braces around a few conditional statements.

 drivers/staging/erofs/inode.c     |  4 ++--
 drivers/staging/erofs/unzip_vle.c | 12 ++++++------
 drivers/staging/erofs/xattr.h     |  6 +++---
 3 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.7.4

[PATCH 1/2] staging/erofs/xattr.h: Fixed misaligned function arguments.

[Caitlyn]

Indented some function arguments to fix checkpath warnings.

Signed-off-by: Caitlyn <caitlynannefinn@gmail.com>
---
 drivers/staging/erofs/xattr.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/erofs/xattr.h b/drivers/staging/erofs/xattr.h
index 35ba5ac..d86f5cd 100644
--- a/drivers/staging/erofs/xattr.h
+++ b/drivers/staging/erofs/xattr.h
@@ -74,14 +74,14 @@ int erofs_getxattr(struct inode *, int, const char *, void *, size_t);
 ssize_t erofs_listxattr(struct dentry *, char *, size_t);
 #else
 static int __maybe_unused erofs_getxattr(struct inode *inode, int index,
-	const char *name,
-	void *buffer, size_t buffer_size)
+					 const char *name, void *buffer,
+					 size_t buffer_size)
 {
 	return -ENOTSUPP;
 }
 
 static ssize_t __maybe_unused erofs_listxattr(struct dentry *dentry,
-	char *buffer, size_t buffer_size)
+					      char *buffer, size_t buffer_size)
 {
 	return -ENOTSUPP;
 }
-- 
2.7.4

[PATCH 2/2] staging/erofs: Balanced braces around a few conditional statements.

[Caitlyn]

Balanced braces to fix some checkpath warnings in inode.c and
unzip_vle.c

Signed-off-by: Caitlyn <caitlynannefinn@gmail.com>
---
 drivers/staging/erofs/inode.c     |  4 ++--
 drivers/staging/erofs/unzip_vle.c | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 4c3d8bf..8de6fcd 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -278,9 +278,9 @@ struct inode *erofs_iget(struct super_block *sb,
 		vi->nid = nid;
 
 		err = fill_inode(inode, isdir);
-		if (likely(!err))
+		if (likely(!err)) {
 			unlock_new_inode(inode);
-		else {
+		} else {
 			iget_failed(inode);
 			inode = ERR_PTR(err);
 		}
diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
index f0dab81..f431614 100644
--- a/drivers/staging/erofs/unzip_vle.c
+++ b/drivers/staging/erofs/unzip_vle.c
@@ -915,21 +915,21 @@ static int z_erofs_vle_unzip(struct super_block *sb,
 	mutex_lock(&work->lock);
 	nr_pages = work->nr_pages;
 
-	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
+	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES)) {
 		pages = pages_onstack;
-	else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
-		 mutex_trylock(&z_pagemap_global_lock))
+	} else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
+		 mutex_trylock(&z_pagemap_global_lock)) {
 		pages = z_pagemap_global;
-	else {
+	} else {
 repeat:
 		pages = kvmalloc_array(nr_pages, sizeof(struct page *),
 				       GFP_KERNEL);
 
 		/* fallback to global pagemap for the lowmem scenario */
 		if (unlikely(!pages)) {
-			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES)
+			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES) {
 				goto repeat;
-			else {
+			} else {
 				mutex_lock(&z_pagemap_global_lock);
 				pages = z_pagemap_global;
 			}
-- 
2.7.4

Re: [PATCH 0/2] Submitting my first patch series (Checkpatch fixes)

[Gao Xiang]

Hi Caitlyn,

On Tue, Aug 20, 2019 at 08:18:18PM -0400, Caitlyn wrote:
> Hello!
> 
> This patch series cleans up some checkpatch fixes in erofs. The patches
> include balancing conditional braces and fixing some indentation. No testing
> done, all patches build and checkpath cleanly.

I think you need to work on the latest staging tree or linux-next.
This patchset cannot be applied (there is the only valid place in inode.c,
I will reply in the following patch.)

Thanks,
Gao Xiang

> 
> Caitlyn (2):
>   staging/erofs/xattr.h: Fixed misaligned function arguments.
>   staging/erofs: Balanced braces around a few conditional statements.
> 
>  drivers/staging/erofs/inode.c     |  4 ++--
>  drivers/staging/erofs/unzip_vle.c | 12 ++++++------
>  drivers/staging/erofs/xattr.h     |  6 +++---
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> -- 
> 2.7.4
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging/erofs: Balanced braces around a few conditional statements.

[Gao Xiang]

On Tue, Aug 20, 2019 at 08:18:20PM -0400, Caitlyn wrote:
> Balanced braces to fix some checkpath warnings in inode.c and
> unzip_vle.c
> 
> Signed-off-by: Caitlyn <caitlynannefinn@gmail.com>
> ---
>  drivers/staging/erofs/inode.c     |  4 ++--
>  drivers/staging/erofs/unzip_vle.c | 12 ++++++------
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
> index 4c3d8bf..8de6fcd 100644
> --- a/drivers/staging/erofs/inode.c
> +++ b/drivers/staging/erofs/inode.c
> @@ -278,9 +278,9 @@ struct inode *erofs_iget(struct super_block *sb,
>  		vi->nid = nid;
>  
>  		err = fill_inode(inode, isdir);
> -		if (likely(!err))
> +		if (likely(!err)) {
>  			unlock_new_inode(inode);

The only valid place is here.

Thanks,
Gao Xiang

> -		else {
> +		} else {
>  			iget_failed(inode);
>  			inode = ERR_PTR(err);
>  		}
> diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
> index f0dab81..f431614 100644
> --- a/drivers/staging/erofs/unzip_vle.c
> +++ b/drivers/staging/erofs/unzip_vle.c
> @@ -915,21 +915,21 @@ static int z_erofs_vle_unzip(struct super_block *sb,
>  	mutex_lock(&work->lock);
>  	nr_pages = work->nr_pages;
>  
> -	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
> +	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES)) {
>  		pages = pages_onstack;
> -	else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> -		 mutex_trylock(&z_pagemap_global_lock))
> +	} else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> +		 mutex_trylock(&z_pagemap_global_lock)) {
>  		pages = z_pagemap_global;
> -	else {
> +	} else {
>  repeat:
>  		pages = kvmalloc_array(nr_pages, sizeof(struct page *),
>  				       GFP_KERNEL);
>  
>  		/* fallback to global pagemap for the lowmem scenario */
>  		if (unlikely(!pages)) {
> -			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES)
> +			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES) {
>  				goto repeat;
> -			else {
> +			} else {
>  				mutex_lock(&z_pagemap_global_lock);
>  				pages = z_pagemap_global;
>  			}
> -- 
> 2.7.4
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging/erofs/xattr.h: Fixed misaligned function arguments.

[Greg Kroah-Hartman]

On Tue, Aug 20, 2019 at 08:18:19PM -0400, Caitlyn wrote:
> Indented some function arguments to fix checkpath warnings.
> 
> Signed-off-by: Caitlyn <caitlynannefinn@gmail.com>
> ---
>  drivers/staging/erofs/xattr.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- It looks like you did not use your "real" name for the patch on either
  the Signed-off-by: line, or the From: line (both of which have to
  match).  Please read the kernel file, Documentation/SubmittingPatches
  for how to do this correctly.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH 2/2] staging/erofs: Balanced braces around a few conditional statements.

[Joe Perches]

On Tue, 2019-08-20 at 20:18 -0400, Caitlyn wrote:
> Balanced braces to fix some checkpath warnings in inode.c and
> unzip_vle.c
[]
> diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
[]
> @@ -915,21 +915,21 @@ static int z_erofs_vle_unzip(struct super_block *sb,
>  	mutex_lock(&work->lock);
>  	nr_pages = work->nr_pages;
>  
> -	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
> +	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES)) {
>  		pages = pages_onstack;
> -	else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> -		 mutex_trylock(&z_pagemap_global_lock))
> +	} else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> +		 mutex_trylock(&z_pagemap_global_lock)) {

Extra space after tab

>  		pages = z_pagemap_global;
> -	else {
> +	} else {
>  repeat:
>  		pages = kvmalloc_array(nr_pages, sizeof(struct page *),
>  				       GFP_KERNEL);
>  
>  		/* fallback to global pagemap for the lowmem scenario */
>  		if (unlikely(!pages)) {
> -			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES)
> +			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES) {
>  				goto repeat;
> -			else {
> +			} else {

Unnecessary else

>  				mutex_lock(&z_pagemap_global_lock);
>  				pages = z_pagemap_global;
>  			}

Re: [PATCH 2/2] staging/erofs: Balanced braces around a few conditional statements.

[Gao Xiang]

On Tue, Aug 20, 2019 at 07:26:46PM -0700, Joe Perches wrote:
> On Tue, 2019-08-20 at 20:18 -0400, Caitlyn wrote:
> > Balanced braces to fix some checkpath warnings in inode.c and
> > unzip_vle.c
> []
> > diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
> []
> > @@ -915,21 +915,21 @@ static int z_erofs_vle_unzip(struct super_block *sb,
> >  	mutex_lock(&work->lock);
> >  	nr_pages = work->nr_pages;
> >  
> > -	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
> > +	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES)) {
> >  		pages = pages_onstack;
> > -	else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > -		 mutex_trylock(&z_pagemap_global_lock))
> > +	} else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > +		 mutex_trylock(&z_pagemap_global_lock)) {
> 
> Extra space after tab

There is actually balanced braces in linux-next.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/staging/erofs/zdata.c#n762

> 
> >  		pages = z_pagemap_global;
> > -	else {
> > +	} else {
> >  repeat:
> >  		pages = kvmalloc_array(nr_pages, sizeof(struct page *),
> >  				       GFP_KERNEL);
> >  
> >  		/* fallback to global pagemap for the lowmem scenario */
> >  		if (unlikely(!pages)) {
> > -			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES)
> > +			if (nr_pages > Z_EROFS_VLE_VMAP_GLOBAL_PAGES) {
> >  				goto repeat;
> > -			else {
> > +			} else {
> 
> Unnecessary else

There is not the "goto repeat" in linux-next anymore.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/staging/erofs/zdata.c#n765

Thanks,
Gao Xiang

> 
> >  				mutex_lock(&z_pagemap_global_lock);
> >  				pages = z_pagemap_global;
> >  			}
> 
> 

Re: [PATCH 2/2] staging/erofs: Balanced braces around a few conditional statements.

[Tobin C. Harding]

On Wed, Aug 21, 2019 at 10:31:22AM +0800, Gao Xiang wrote:
> On Tue, Aug 20, 2019 at 07:26:46PM -0700, Joe Perches wrote:
> > On Tue, 2019-08-20 at 20:18 -0400, Caitlyn wrote:
> > > Balanced braces to fix some checkpath warnings in inode.c and
> > > unzip_vle.c
> > []
> > > diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
> > []
> > > @@ -915,21 +915,21 @@ static int z_erofs_vle_unzip(struct super_block *sb,
> > >  	mutex_lock(&work->lock);
> > >  	nr_pages = work->nr_pages;
> > >  
> > > -	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
> > > +	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES)) {
> > >  		pages = pages_onstack;
> > > -	else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > > -		 mutex_trylock(&z_pagemap_global_lock))
> > > +	} else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > > +		 mutex_trylock(&z_pagemap_global_lock)) {
> > 
> > Extra space after tab
> 
> There is actually balanced braces in linux-next.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/staging/erofs/zdata.c#n762

Which tree did these changes go in through please Gao?  I believe
Caitlyn was working off of the staging-next branch of Greg's staging
tree.

thanks,
Tobin.

Re: [PATCH 2/2] staging/erofs: Balanced braces around a few conditional statements.

[Gao Xiang]

Hi Tobin,

On Wed, Aug 21, 2019 at 08:13:35AM -0700, Tobin C. Harding wrote:
> On Wed, Aug 21, 2019 at 10:31:22AM +0800, Gao Xiang wrote:
> > On Tue, Aug 20, 2019 at 07:26:46PM -0700, Joe Perches wrote:
> > > On Tue, 2019-08-20 at 20:18 -0400, Caitlyn wrote:
> > > > Balanced braces to fix some checkpath warnings in inode.c and
> > > > unzip_vle.c
> > > []
> > > > diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
> > > []
> > > > @@ -915,21 +915,21 @@ static int z_erofs_vle_unzip(struct super_block *sb,
> > > >  	mutex_lock(&work->lock);
> > > >  	nr_pages = work->nr_pages;
> > > >  
> > > > -	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
> > > > +	if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES)) {
> > > >  		pages = pages_onstack;
> > > > -	else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > > > -		 mutex_trylock(&z_pagemap_global_lock))
> > > > +	} else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > > > +		 mutex_trylock(&z_pagemap_global_lock)) {
> > > 
> > > Extra space after tab
> > 
> > There is actually balanced braces in linux-next.
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/staging/erofs/zdata.c#n762
> 
> Which tree did these changes go in through please Gao?  I believe
> Caitlyn was working off of the staging-next branch of Greg's staging
> tree.

I don't think so, the reason is that unzip_vle.c was renamed to zdata.c
months ago, see:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/erofs?h=staging-next

so I think the patch is outdated when I first look at it.

Thanks,
Gao Xiang

> 
> thanks,
> Tobin.

Re: [PATCH 2/2] staging/erofs: Balanced braces around a few conditional statements.

[Caitlyn Finn]

On Wed, Aug 21, 2019 at 11:52 AM Gao Xiang <hsiangkao@aol.com> wrote:
>
> Hi Tobin,
>
> On Wed, Aug 21, 2019 at 08:13:35AM -0700, Tobin C. Harding wrote:
> > On Wed, Aug 21, 2019 at 10:31:22AM +0800, Gao Xiang wrote:
> > > On Tue, Aug 20, 2019 at 07:26:46PM -0700, Joe Perches wrote:
> > > > On Tue, 2019-08-20 at 20:18 -0400, Caitlyn wrote:
> > > > > Balanced braces to fix some checkpath warnings in inode.c and
> > > > > unzip_vle.c
> > > > []
> > > > > diff --git a/drivers/staging/erofs/unzip_vle.c b/drivers/staging/erofs/unzip_vle.c
> > > > []
> > > > > @@ -915,21 +915,21 @@ static int z_erofs_vle_unzip(struct super_block *sb,
> > > > >         mutex_lock(&work->lock);
> > > > >         nr_pages = work->nr_pages;
> > > > >
> > > > > -       if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES))
> > > > > +       if (likely(nr_pages <= Z_EROFS_VLE_VMAP_ONSTACK_PAGES)) {
> > > > >                 pages = pages_onstack;
> > > > > -       else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > > > > -                mutex_trylock(&z_pagemap_global_lock))
> > > > > +       } else if (nr_pages <= Z_EROFS_VLE_VMAP_GLOBAL_PAGES &&
> > > > > +                mutex_trylock(&z_pagemap_global_lock)) {
> > > >
> > > > Extra space after tab
> > >
> > > There is actually balanced braces in linux-next.
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/staging/erofs/zdata.c#n762
> >
> > Which tree did these changes go in through please Gao?  I believe
> > Caitlyn was working off of the staging-next branch of Greg's staging
> > tree.
>
> I don't think so, the reason is that unzip_vle.c was renamed to zdata.c
> months ago, see:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/tree/drivers/staging/erofs?h=staging-next
>
> so I think the patch is outdated when I first look at it.
>
> Thanks,
> Gao Xiang

Gao,

I see now that I was on an outdated revision (Linux 5.3-rc4) of the
staging-next branch of Greg's staging tree, from
before that change was merged. I'll be certain that I'm fully
up-to-date before submitting future patches.

Thanks all for your time and assistance, and Gao and Joe for the
review comments as well, I'll review and submit
an appropriate patch series at a later time.

Thanks,
Caitlyn Finn