linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] JFS: fix memleak in jfs_mount
@ 2021-08-18 10:25 Dongliang Mu
  2021-08-30  8:15 ` Dongliang Mu
  2021-09-03 21:51 ` Dave Kleikamp
  0 siblings, 2 replies; 4+ messages in thread
From: Dongliang Mu @ 2021-08-18 10:25 UTC (permalink / raw)
  To: Dave Kleikamp; +Cc: Dongliang Mu, jfs-discussion, linux-kernel

In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
the following code does not free ipaimap2 allocated by diReadSpecial.

Fix this by refactoring the error handling code of jfs_mount. To be
specific, modify the lable name and free ipaimap2 when the above error
ocurrs.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
---
 fs/jfs/jfs_mount.c | 53 +++++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 31 deletions(-)

diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
index 5d7d7170c03c..638a4ecc4069 100644
--- a/fs/jfs/jfs_mount.c
+++ b/fs/jfs/jfs_mount.c
@@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
 	 * (initialize mount inode from the superblock)
 	 */
 	if ((rc = chkSuper(sb))) {
-		goto errout20;
+		return rc;
 	}
 
 	ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
 	if (ipaimap == NULL) {
 		jfs_err("jfs_mount: Failed to read AGGREGATE_I");
 		rc = -EIO;
-		goto errout20;
+		goto out;
 	}
 	sbi->ipaimap = ipaimap;
 
@@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
 	 */
 	if ((rc = diMount(ipaimap))) {
 		jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
-		goto errout21;
+		goto err_ipaimap;
 	}
 
 	/*
@@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
 	ipbmap = diReadSpecial(sb, BMAP_I, 0);
 	if (ipbmap == NULL) {
 		rc = -EIO;
-		goto errout22;
+		goto err_umount_ipaimap;
 	}
 
 	jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
@@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
 	 */
 	if ((rc = dbMount(ipbmap))) {
 		jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
-		goto errout22;
+		goto err_ipbmap;
 	}
 
 	/*
@@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
 		if (!ipaimap2) {
 			jfs_err("jfs_mount: Failed to read AGGREGATE_I");
 			rc = -EIO;
-			goto errout35;
+			goto err_umount_ipbmap;
 		}
 		sbi->ipaimap2 = ipaimap2;
 
@@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
 		if ((rc = diMount(ipaimap2))) {
 			jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
 				rc);
-			goto errout35;
+			goto err_ipaimap2;
 		}
 	} else
 		/* Secondary aggregate inode table is not valid */
@@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
 		jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
 		/* open fileset secondary inode allocation map */
 		rc = -EIO;
-		goto errout40;
+		goto err_umount_ipaimap2;
 	}
 	jfs_info("jfs_mount: ipimap:0x%p", ipimap);
 
@@ -178,41 +178,32 @@ int jfs_mount(struct super_block *sb)
 	/* initialize fileset inode allocation map */
 	if ((rc = diMount(ipimap))) {
 		jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
-		goto errout41;
+		goto err_ipimap;
 	}
 
-	goto out;
+	return rc;
 
 	/*
 	 *	unwind on error
 	 */
-      errout41:		/* close fileset inode allocation map inode */
+err_ipimap:
+	/* close fileset inode allocation map inode */
 	diFreeSpecial(ipimap);
-
-      errout40:		/* fileset closed */
-
+err_umount_ipaimap2:
 	/* close secondary aggregate inode allocation map */
-	if (ipaimap2) {
-		diUnmount(ipaimap2, 1);
-		diFreeSpecial(ipaimap2);
-	}
-
-      errout35:
-
-	/* close aggregate block allocation map */
+	if (ipaimap2) diUnmount(ipaimap2, 1);
+err_ipaimap2:
+	/* close aggregate inodes */
+	if (ipaimap2) diFreeSpecial(ipaimap2);
+err_umount_ipbmap:	/* close aggregate block allocation map */
 	dbUnmount(ipbmap, 1);
+err_ipbmap:		/* close aggregate inodes */
 	diFreeSpecial(ipbmap);
-
-      errout22:		/* close aggregate inode allocation map */
-
+err_umount_ipaimap:	/* close aggregate inode allocation map */
 	diUnmount(ipaimap, 1);
-
-      errout21:		/* close aggregate inodes */
+err_ipaimap:		/* close aggregate inodes */
 	diFreeSpecial(ipaimap);
-      errout20:		/* aggregate closed */
-
-      out:
-
+out:
 	if (rc)
 		jfs_err("Mount JFS Failure: %d", rc);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] JFS: fix memleak in jfs_mount
  2021-08-18 10:25 [PATCH] JFS: fix memleak in jfs_mount Dongliang Mu
@ 2021-08-30  8:15 ` Dongliang Mu
  2021-09-03 21:51 ` Dave Kleikamp
  1 sibling, 0 replies; 4+ messages in thread
From: Dongliang Mu @ 2021-08-30  8:15 UTC (permalink / raw)
  To: Dave Kleikamp; +Cc: jfs-discussion, linux-kernel

On Wed, Aug 18, 2021 at 6:26 PM Dongliang Mu <mudongliangabcd@gmail.com> wrote:
>
> In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
> the following code does not free ipaimap2 allocated by diReadSpecial.
>
> Fix this by refactoring the error handling code of jfs_mount. To be
> specific, modify the lable name and free ipaimap2 when the above error
> ocurrs.
>

Hi maintainers,

any comment on this patch?

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
>  fs/jfs/jfs_mount.c | 53 +++++++++++++++++++---------------------------
>  1 file changed, 22 insertions(+), 31 deletions(-)
>
> diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
> index 5d7d7170c03c..638a4ecc4069 100644
> --- a/fs/jfs/jfs_mount.c
> +++ b/fs/jfs/jfs_mount.c
> @@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
>          * (initialize mount inode from the superblock)
>          */
>         if ((rc = chkSuper(sb))) {
> -               goto errout20;
> +               return rc;
>         }
>
>         ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
>         if (ipaimap == NULL) {
>                 jfs_err("jfs_mount: Failed to read AGGREGATE_I");
>                 rc = -EIO;
> -               goto errout20;
> +               goto out;
>         }
>         sbi->ipaimap = ipaimap;
>
> @@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
>          */
>         if ((rc = diMount(ipaimap))) {
>                 jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
> -               goto errout21;
> +               goto err_ipaimap;
>         }
>
>         /*
> @@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
>         ipbmap = diReadSpecial(sb, BMAP_I, 0);
>         if (ipbmap == NULL) {
>                 rc = -EIO;
> -               goto errout22;
> +               goto err_umount_ipaimap;
>         }
>
>         jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
> @@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
>          */
>         if ((rc = dbMount(ipbmap))) {
>                 jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
> -               goto errout22;
> +               goto err_ipbmap;
>         }
>
>         /*
> @@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
>                 if (!ipaimap2) {
>                         jfs_err("jfs_mount: Failed to read AGGREGATE_I");
>                         rc = -EIO;
> -                       goto errout35;
> +                       goto err_umount_ipbmap;
>                 }
>                 sbi->ipaimap2 = ipaimap2;
>
> @@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
>                 if ((rc = diMount(ipaimap2))) {
>                         jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
>                                 rc);
> -                       goto errout35;
> +                       goto err_ipaimap2;
>                 }
>         } else
>                 /* Secondary aggregate inode table is not valid */
> @@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
>                 jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
>                 /* open fileset secondary inode allocation map */
>                 rc = -EIO;
> -               goto errout40;
> +               goto err_umount_ipaimap2;
>         }
>         jfs_info("jfs_mount: ipimap:0x%p", ipimap);
>
> @@ -178,41 +178,32 @@ int jfs_mount(struct super_block *sb)
>         /* initialize fileset inode allocation map */
>         if ((rc = diMount(ipimap))) {
>                 jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
> -               goto errout41;
> +               goto err_ipimap;
>         }
>
> -       goto out;
> +       return rc;
>
>         /*
>          *      unwind on error
>          */
> -      errout41:                /* close fileset inode allocation map inode */
> +err_ipimap:
> +       /* close fileset inode allocation map inode */
>         diFreeSpecial(ipimap);
> -
> -      errout40:                /* fileset closed */
> -
> +err_umount_ipaimap2:
>         /* close secondary aggregate inode allocation map */
> -       if (ipaimap2) {
> -               diUnmount(ipaimap2, 1);
> -               diFreeSpecial(ipaimap2);
> -       }
> -
> -      errout35:
> -
> -       /* close aggregate block allocation map */
> +       if (ipaimap2) diUnmount(ipaimap2, 1);
> +err_ipaimap2:
> +       /* close aggregate inodes */
> +       if (ipaimap2) diFreeSpecial(ipaimap2);
> +err_umount_ipbmap:     /* close aggregate block allocation map */
>         dbUnmount(ipbmap, 1);
> +err_ipbmap:            /* close aggregate inodes */
>         diFreeSpecial(ipbmap);
> -
> -      errout22:                /* close aggregate inode allocation map */
> -
> +err_umount_ipaimap:    /* close aggregate inode allocation map */
>         diUnmount(ipaimap, 1);
> -
> -      errout21:                /* close aggregate inodes */
> +err_ipaimap:           /* close aggregate inodes */
>         diFreeSpecial(ipaimap);
> -      errout20:                /* aggregate closed */
> -
> -      out:
> -
> +out:
>         if (rc)
>                 jfs_err("Mount JFS Failure: %d", rc);
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] JFS: fix memleak in jfs_mount
  2021-08-18 10:25 [PATCH] JFS: fix memleak in jfs_mount Dongliang Mu
  2021-08-30  8:15 ` Dongliang Mu
@ 2021-09-03 21:51 ` Dave Kleikamp
  2021-09-04  2:26   ` Dongliang Mu
  1 sibling, 1 reply; 4+ messages in thread
From: Dave Kleikamp @ 2021-09-03 21:51 UTC (permalink / raw)
  To: Dongliang Mu; +Cc: jfs-discussion, linux-kernel

Thanks for waiting. I have just a couple comments, but this looks good. 
I appreciate the clean up.

On 8/18/21 5:25 AM, Dongliang Mu wrote:
> In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
> the following code does not free ipaimap2 allocated by diReadSpecial.
> 
> Fix this by refactoring the error handling code of jfs_mount. To be
> specific, modify the lable name and free ipaimap2 when the above error
> ocurrs.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> ---
>   fs/jfs/jfs_mount.c | 53 +++++++++++++++++++---------------------------
>   1 file changed, 22 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
> index 5d7d7170c03c..638a4ecc4069 100644
> --- a/fs/jfs/jfs_mount.c
> +++ b/fs/jfs/jfs_mount.c
> @@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
>   	 * (initialize mount inode from the superblock)
>   	 */
>   	if ((rc = chkSuper(sb))) {
> -		goto errout20;
> +		return rc;

This may have been intentional, but it isn't mentioned in the patch header.

If chkSuper fails, we will no longer call
	jfs_err("Mount JFS Failure: %d", rc);

I don't necessarily see this as a bad thing. In many cases, chkSuper 
prints a more helpful message. In the case where it silently fails, it's 
not even recognizing the superblock as a supported version of JFS and 
this message isn't particularly helpful. In fact this jfs_err() 
statement might be best deleted in its entirety.

>   	}
>   
>   	ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
>   	if (ipaimap == NULL) {
>   		jfs_err("jfs_mount: Failed to read AGGREGATE_I");
>   		rc = -EIO;
> -		goto errout20;
> +		goto out;
>   	}
>   	sbi->ipaimap = ipaimap;
>   
> @@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
>   	 */
>   	if ((rc = diMount(ipaimap))) {
>   		jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
> -		goto errout21;
> +		goto err_ipaimap;
>   	}
>   
>   	/*
> @@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
>   	ipbmap = diReadSpecial(sb, BMAP_I, 0);
>   	if (ipbmap == NULL) {
>   		rc = -EIO;
> -		goto errout22;
> +		goto err_umount_ipaimap;
>   	}
>   
>   	jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
> @@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
>   	 */
>   	if ((rc = dbMount(ipbmap))) {
>   		jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
> -		goto errout22;
> +		goto err_ipbmap;
>   	}
>   
>   	/*
> @@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
>   		if (!ipaimap2) {
>   			jfs_err("jfs_mount: Failed to read AGGREGATE_I");
>   			rc = -EIO;
> -			goto errout35;
> +			goto err_umount_ipbmap;
>   		}
>   		sbi->ipaimap2 = ipaimap2;
>   
> @@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
>   		if ((rc = diMount(ipaimap2))) {
>   			jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
>   				rc);
> -			goto errout35;
> +			goto err_ipaimap2;
>   		}
>   	} else
>   		/* Secondary aggregate inode table is not valid */
> @@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
>   		jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
>   		/* open fileset secondary inode allocation map */
>   		rc = -EIO;
> -		goto errout40;
> +		goto err_umount_ipaimap2;
>   	}
>   	jfs_info("jfs_mount: ipimap:0x%p", ipimap);
>   
> @@ -178,41 +178,32 @@ int jfs_mount(struct super_block *sb)
>   	/* initialize fileset inode allocation map */
>   	if ((rc = diMount(ipimap))) {
>   		jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
> -		goto errout41;
> +		goto err_ipimap;
>   	}
>   
> -	goto out;
> +	return rc;
>   
>   	/*
>   	 *	unwind on error
>   	 */
> -      errout41:		/* close fileset inode allocation map inode */
> +err_ipimap:
> +	/* close fileset inode allocation map inode */
>   	diFreeSpecial(ipimap);
> -
> -      errout40:		/* fileset closed */
> -
> +err_umount_ipaimap2:
>   	/* close secondary aggregate inode allocation map */
> -	if (ipaimap2) {
> -		diUnmount(ipaimap2, 1);
> -		diFreeSpecial(ipaimap2);
> -	}
> -
> -      errout35:
> -
> -	/* close aggregate block allocation map */
> +	if (ipaimap2) diUnmount(ipaimap2, 1);

Coding style: this should be split between two lines:
	if (ipaimap2)
		diUnmount(ipaimap2, 1);

> +err_ipaimap2:
> +	/* close aggregate inodes */
> +	if (ipaimap2) diFreeSpecial(ipaimap2);

Same here.

> +err_umount_ipbmap:	/* close aggregate block allocation map */
>   	dbUnmount(ipbmap, 1);
> +err_ipbmap:		/* close aggregate inodes */
>   	diFreeSpecial(ipbmap);
> -
> -      errout22:		/* close aggregate inode allocation map */
> -
> +err_umount_ipaimap:	/* close aggregate inode allocation map */
>   	diUnmount(ipaimap, 1);
> -
> -      errout21:		/* close aggregate inodes */
> +err_ipaimap:		/* close aggregate inodes */
>   	diFreeSpecial(ipaimap);
> -      errout20:		/* aggregate closed */
> -
> -      out:
> -
> +out:
>   	if (rc)
>   		jfs_err("Mount JFS Failure: %d", rc);
>   
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] JFS: fix memleak in jfs_mount
  2021-09-03 21:51 ` Dave Kleikamp
@ 2021-09-04  2:26   ` Dongliang Mu
  0 siblings, 0 replies; 4+ messages in thread
From: Dongliang Mu @ 2021-09-04  2:26 UTC (permalink / raw)
  To: Dave Kleikamp; +Cc: jfs-discussion, linux-kernel

On Sat, Sep 4, 2021 at 5:51 AM Dave Kleikamp <dave.kleikamp@oracle.com> wrote:
>
> Thanks for waiting. I have just a couple comments, but this looks good.
> I appreciate the clean up.
>
> On 8/18/21 5:25 AM, Dongliang Mu wrote:
> > In jfs_mount, when diMount(ipaimap2) fails, it goes to errout35. However,
> > the following code does not free ipaimap2 allocated by diReadSpecial.
> >
> > Fix this by refactoring the error handling code of jfs_mount. To be
> > specific, modify the lable name and free ipaimap2 when the above error
> > ocurrs.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > ---
> >   fs/jfs/jfs_mount.c | 53 +++++++++++++++++++---------------------------
> >   1 file changed, 22 insertions(+), 31 deletions(-)
> >
> > diff --git a/fs/jfs/jfs_mount.c b/fs/jfs/jfs_mount.c
> > index 5d7d7170c03c..638a4ecc4069 100644
> > --- a/fs/jfs/jfs_mount.c
> > +++ b/fs/jfs/jfs_mount.c
> > @@ -81,14 +81,14 @@ int jfs_mount(struct super_block *sb)
> >        * (initialize mount inode from the superblock)
> >        */
> >       if ((rc = chkSuper(sb))) {
> > -             goto errout20;
> > +             return rc;
>
> This may have been intentional, but it isn't mentioned in the patch header.
>
> If chkSuper fails, we will no longer call
>         jfs_err("Mount JFS Failure: %d", rc);
>
> I don't necessarily see this as a bad thing. In many cases, chkSuper
> prints a more helpful message. In the case where it silently fails, it's
> not even recognizing the superblock as a supported version of JFS and
> this message isn't particularly helpful. In fact this jfs_err()
> statement might be best deleted in its entirety.

I see. Let me change "return rc" to "goto out", the same as "goto errout20".

This will put back the jfs_err message output.

>
> >       }
> >
> >       ipaimap = diReadSpecial(sb, AGGREGATE_I, 0);
> >       if (ipaimap == NULL) {
> >               jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> >               rc = -EIO;
> > -             goto errout20;
> > +             goto out;
> >       }
> >       sbi->ipaimap = ipaimap;
> >
> > @@ -99,7 +99,7 @@ int jfs_mount(struct super_block *sb)
> >        */
> >       if ((rc = diMount(ipaimap))) {
> >               jfs_err("jfs_mount: diMount(ipaimap) failed w/rc = %d", rc);
> > -             goto errout21;
> > +             goto err_ipaimap;
> >       }
> >
> >       /*
> > @@ -108,7 +108,7 @@ int jfs_mount(struct super_block *sb)
> >       ipbmap = diReadSpecial(sb, BMAP_I, 0);
> >       if (ipbmap == NULL) {
> >               rc = -EIO;
> > -             goto errout22;
> > +             goto err_umount_ipaimap;
> >       }
> >
> >       jfs_info("jfs_mount: ipbmap:0x%p", ipbmap);
> > @@ -120,7 +120,7 @@ int jfs_mount(struct super_block *sb)
> >        */
> >       if ((rc = dbMount(ipbmap))) {
> >               jfs_err("jfs_mount: dbMount failed w/rc = %d", rc);
> > -             goto errout22;
> > +             goto err_ipbmap;
> >       }
> >
> >       /*
> > @@ -139,7 +139,7 @@ int jfs_mount(struct super_block *sb)
> >               if (!ipaimap2) {
> >                       jfs_err("jfs_mount: Failed to read AGGREGATE_I");
> >                       rc = -EIO;
> > -                     goto errout35;
> > +                     goto err_umount_ipbmap;
> >               }
> >               sbi->ipaimap2 = ipaimap2;
> >
> > @@ -151,7 +151,7 @@ int jfs_mount(struct super_block *sb)
> >               if ((rc = diMount(ipaimap2))) {
> >                       jfs_err("jfs_mount: diMount(ipaimap2) failed, rc = %d",
> >                               rc);
> > -                     goto errout35;
> > +                     goto err_ipaimap2;
> >               }
> >       } else
> >               /* Secondary aggregate inode table is not valid */
> > @@ -168,7 +168,7 @@ int jfs_mount(struct super_block *sb)
> >               jfs_err("jfs_mount: Failed to read FILESYSTEM_I");
> >               /* open fileset secondary inode allocation map */
> >               rc = -EIO;
> > -             goto errout40;
> > +             goto err_umount_ipaimap2;
> >       }
> >       jfs_info("jfs_mount: ipimap:0x%p", ipimap);
> >
> > @@ -178,41 +178,32 @@ int jfs_mount(struct super_block *sb)
> >       /* initialize fileset inode allocation map */
> >       if ((rc = diMount(ipimap))) {
> >               jfs_err("jfs_mount: diMount failed w/rc = %d", rc);
> > -             goto errout41;
> > +             goto err_ipimap;
> >       }
> >
> > -     goto out;
> > +     return rc;
> >
> >       /*
> >        *      unwind on error
> >        */
> > -      errout41:              /* close fileset inode allocation map inode */
> > +err_ipimap:
> > +     /* close fileset inode allocation map inode */
> >       diFreeSpecial(ipimap);
> > -
> > -      errout40:              /* fileset closed */
> > -
> > +err_umount_ipaimap2:
> >       /* close secondary aggregate inode allocation map */
> > -     if (ipaimap2) {
> > -             diUnmount(ipaimap2, 1);
> > -             diFreeSpecial(ipaimap2);
> > -     }
> > -
> > -      errout35:
> > -
> > -     /* close aggregate block allocation map */
> > +     if (ipaimap2) diUnmount(ipaimap2, 1);
>
> Coding style: this should be split between two lines:
>         if (ipaimap2)
>                 diUnmount(ipaimap2, 1);
>
> > +err_ipaimap2:
> > +     /* close aggregate inodes */
> > +     if (ipaimap2) diFreeSpecial(ipaimap2);
>
> Same here.

Will modify it in the v2 version.

>
> > +err_umount_ipbmap:   /* close aggregate block allocation map */
> >       dbUnmount(ipbmap, 1);
> > +err_ipbmap:          /* close aggregate inodes */
> >       diFreeSpecial(ipbmap);
> > -
> > -      errout22:              /* close aggregate inode allocation map */
> > -
> > +err_umount_ipaimap:  /* close aggregate inode allocation map */
> >       diUnmount(ipaimap, 1);
> > -
> > -      errout21:              /* close aggregate inodes */
> > +err_ipaimap:         /* close aggregate inodes */
> >       diFreeSpecial(ipaimap);
> > -      errout20:              /* aggregate closed */
> > -
> > -      out:
> > -
> > +out:
> >       if (rc)
> >               jfs_err("Mount JFS Failure: %d", rc);
> >
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-04  2:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18 10:25 [PATCH] JFS: fix memleak in jfs_mount Dongliang Mu
2021-08-30  8:15 ` Dongliang Mu
2021-09-03 21:51 ` Dave Kleikamp
2021-09-04  2:26   ` Dongliang Mu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).