* Re: UBIFS: make space fixup work in the remount case [not found] <20130415095924.GA26544@elgon.mountain> @ 2013-04-15 10:11 ` Artem Bityutskiy 2013-04-15 10:19 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Artem Bityutskiy @ 2013-04-15 10:11 UTC (permalink / raw) To: Dan Carpenter; +Cc: linux-mtd On Mon, 2013-04-15 at 12:59 +0300, Dan Carpenter wrote: > 1571 if (c->space_fixup) { > 1572 err = ubifs_fixup_free_space(c); > 1573 if (err) > 1574 return err; > ^^^^^^^^^^ > Shouldn't this be a "goto out?". We're holding the c->umount_mutex. Hi Dan, I fixed this up and answered the reporter: https://patchwork.kernel.org/patch/2284681/ It is now 'goto out'. Thanks! -- Best Regards, Artem Bityutskiy ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: UBIFS: make space fixup work in the remount case 2013-04-15 10:11 ` UBIFS: make space fixup work in the remount case Artem Bityutskiy @ 2013-04-15 10:19 ` Dan Carpenter 2013-04-15 10:39 ` Artem Bityutskiy 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2013-04-15 10:19 UTC (permalink / raw) To: Artem Bityutskiy; +Cc: linux-mtd On Mon, Apr 15, 2013 at 01:11:33PM +0300, Artem Bityutskiy wrote: > On Mon, 2013-04-15 at 12:59 +0300, Dan Carpenter wrote: > > 1571 if (c->space_fixup) { > > 1572 err = ubifs_fixup_free_space(c); > > 1573 if (err) > > 1574 return err; > > ^^^^^^^^^^ > > Shouldn't this be a "goto out?". We're holding the c->umount_mutex. > > Hi Dan, I fixed this up and answered the reporter: > https://patchwork.kernel.org/patch/2284681/ > > It is now 'goto out'. Thanks! Huh... That wasn't who I expected to report it. :P It's good to know that these kind of bugs don't last very long. regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: UBIFS: make space fixup work in the remount case 2013-04-15 10:19 ` Dan Carpenter @ 2013-04-15 10:39 ` Artem Bityutskiy 2014-04-01 13:39 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Artem Bityutskiy @ 2013-04-15 10:39 UTC (permalink / raw) To: Dan Carpenter; +Cc: linux-mtd On Mon, 2013-04-15 at 13:19 +0300, Dan Carpenter wrote: > On Mon, Apr 15, 2013 at 01:11:33PM +0300, Artem Bityutskiy wrote: > > On Mon, 2013-04-15 at 12:59 +0300, Dan Carpenter wrote: > > > 1571 if (c->space_fixup) { > > > 1572 err = ubifs_fixup_free_space(c); > > > 1573 if (err) > > > 1574 return err; > > > ^^^^^^^^^^ > > > Shouldn't this be a "goto out?". We're holding the c->umount_mutex. > > > > Hi Dan, I fixed this up and answered the reporter: > > https://patchwork.kernel.org/patch/2284681/ > > > > It is now 'goto out'. Thanks! > > Huh... That wasn't who I expected to report it. :P It's good to > know that these kind of bugs don't last very long. The story actually was that I noticed this myself after running Aiaiai and fixed it up "in-place", and then several people reported, and I answered to one of them. -- Best Regards, Artem Bityutskiy ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: UBIFS: make space fixup work in the remount case 2013-04-15 10:39 ` Artem Bityutskiy @ 2014-04-01 13:39 ` Dan Carpenter 2014-04-07 10:43 ` Artem Bityutskiy 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2014-04-01 13:39 UTC (permalink / raw) To: Artem Bityutskiy; +Cc: linux-mtd On Mon, Apr 15, 2013 at 01:39:06PM +0300, Artem Bityutskiy wrote: > On Mon, 2013-04-15 at 13:19 +0300, Dan Carpenter wrote: > > On Mon, Apr 15, 2013 at 01:11:33PM +0300, Artem Bityutskiy wrote: > > > On Mon, 2013-04-15 at 12:59 +0300, Dan Carpenter wrote: > > > > 1571 if (c->space_fixup) { > > > > 1572 err = ubifs_fixup_free_space(c); > > > > 1573 if (err) > > > > 1574 return err; > > > > ^^^^^^^^^^ > > > > Shouldn't this be a "goto out?". We're holding the c->umount_mutex. > > > > > > Hi Dan, I fixed this up and answered the reporter: > > > https://patchwork.kernel.org/patch/2284681/ > > > > > > It is now 'goto out'. Thanks! > > > > Huh... That wasn't who I expected to report it. :P It's good to > > know that these kind of bugs don't last very long. > > The story actually was that I noticed this myself after running Aiaiai > and fixed it up "in-place", and then several people reported, and I > answered to one of them. Hi Artem, This showed up in my smatch output again today. Whatever happened with this. I reported it last Jun and last Nov as well apparently. regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: UBIFS: make space fixup work in the remount case 2014-04-01 13:39 ` Dan Carpenter @ 2014-04-07 10:43 ` Artem Bityutskiy 0 siblings, 0 replies; 5+ messages in thread From: Artem Bityutskiy @ 2014-04-07 10:43 UTC (permalink / raw) To: Dan Carpenter; +Cc: linux-mtd On Tue, 2014-04-01 at 16:39 +0300, Dan Carpenter wrote: > On Mon, Apr 15, 2013 at 01:39:06PM +0300, Artem Bityutskiy wrote: > > On Mon, 2013-04-15 at 13:19 +0300, Dan Carpenter wrote: > > > On Mon, Apr 15, 2013 at 01:11:33PM +0300, Artem Bityutskiy wrote: > > > > On Mon, 2013-04-15 at 12:59 +0300, Dan Carpenter wrote: > > > > > 1571 if (c->space_fixup) { > > > > > 1572 err = ubifs_fixup_free_space(c); > > > > > 1573 if (err) > > > > > 1574 return err; > > > > > ^^^^^^^^^^ > > > > > Shouldn't this be a "goto out?". We're holding the c->umount_mutex. > > > > > > > > Hi Dan, I fixed this up and answered the reporter: > > > > https://patchwork.kernel.org/patch/2284681/ > > > > > > > > It is now 'goto out'. Thanks! > > > > > > Huh... That wasn't who I expected to report it. :P It's good to > > > know that these kind of bugs don't last very long. > > > > The story actually was that I noticed this myself after running Aiaiai > > and fixed it up "in-place", and then several people reported, and I > > answered to one of them. > > Hi Artem, > > This showed up in my smatch output again today. Whatever happened with > this. I reported it last Jun and last Nov as well apparently. Hmm, I was sure I fixed this, but it is still there. OK, created and pushed a fix to the master branch. It won't hit linux-next so far, I'll propagate it there once 3.15-rc1 or -rc2 is out. Thanks! From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> Date: Mon, 7 Apr 2014 13:39:06 +0300 Subject: [PATCH] UBIFS: fix remount error path Dan's "smatch" checker found out that there was a bug in the error path of the 'ubifs_remount_rw()' function. Instead of jumping to the "out" label which cleans-things up, we just returned. This patch fixes the problem. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> --- fs/ubifs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 5ded849..7e71ac9 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1556,7 +1556,7 @@ static int ubifs_remount_rw(struct ubifs_info *c) if (c->space_fixup) { err = ubifs_fixup_free_space(c); if (err) - return err; + goto out; } err = check_free_space(c); -- 1.8.5.3 -- Best Regards, Artem Bityutskiy ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-04-07 10:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20130415095924.GA26544@elgon.mountain>
2013-04-15 10:11 ` UBIFS: make space fixup work in the remount case Artem Bityutskiy
2013-04-15 10:19 ` Dan Carpenter
2013-04-15 10:39 ` Artem Bityutskiy
2014-04-01 13:39 ` Dan Carpenter
2014-04-07 10:43 ` Artem Bityutskiy
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).