diff mbox

UBI: fix missing brace control flow

Message ID 1424725642-26270-1-git-send-email-computersforpeace@gmail.com
State Accepted
Headers show

Commit Message

Brian Norris Feb. 23, 2015, 9:07 p.m. UTC
commit 0e707ae79ba3 ("UBI: do propagate positive error codes up") seems
to have produced an unintended change in the control flow here.

Completely untested, but it looks obvious.

Caught by Coverity, which didn't like the indentation. CID 1271184.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
---
Should go into 4.0, I expect.

 drivers/mtd/ubi/eba.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Richard Weinberger Feb. 23, 2015, 9:21 p.m. UTC | #1
Am 23.02.2015 um 22:07 schrieb Brian Norris:
> commit 0e707ae79ba3 ("UBI: do propagate positive error codes up") seems
> to have produced an unintended change in the control flow here.
> 
> Completely untested, but it looks obvious.
> 
> Caught by Coverity, which didn't like the indentation. CID 1271184.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Should go into 4.0, I expect.

Good catch, patch tested and applied!

Thank you Brian!
//richard
Brian Norris March 16, 2015, 6:08 p.m. UTC | #2
On Mon, Feb 23, 2015 at 10:21:53PM +0100, Richard Weinberger wrote:
> Am 23.02.2015 um 22:07 schrieb Brian Norris:
> > commit 0e707ae79ba3 ("UBI: do propagate positive error codes up") seems
> > to have produced an unintended change in the control flow here.
> > 
> > Completely untested, but it looks obvious.
> > 
> > Caught by Coverity, which didn't like the indentation. CID 1271184.
> > 
> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Should go into 4.0, I expect.
> 
> Good catch, patch tested and applied!

Is this going in 4.0? It fixes a typo in a hastily-applied patch that
made it to 4.0-rc1.

I'm also not sure I understand the role of the +linux-next and +master
branches in linux-ubifs.git. Typically 'next' means for the current+1
release (i.e., 4.1), while 'not-next' (i.e., your master branch?) would
be for the current release (4.0). But you have +master based on top of
+linux-next.

Brian
Richard Weinberger March 16, 2015, 6:21 p.m. UTC | #3
Hi!

Am 16.03.2015 um 19:08 schrieb Brian Norris:
> On Mon, Feb 23, 2015 at 10:21:53PM +0100, Richard Weinberger wrote:
>> Am 23.02.2015 um 22:07 schrieb Brian Norris:
>>> commit 0e707ae79ba3 ("UBI: do propagate positive error codes up") seems
>>> to have produced an unintended change in the control flow here.
>>>
>>> Completely untested, but it looks obvious.
>>>
>>> Caught by Coverity, which didn't like the indentation. CID 1271184.
>>>
>>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
>>> Cc: Dan Carpenter <dan.carpenter@oracle.com>
>>> ---
>>> Should go into 4.0, I expect.
>>
>> Good catch, patch tested and applied!
> 
> Is this going in 4.0? It fixes a typo in a hastily-applied patch that
> made it to 4.0-rc1.

That's the plan.

> I'm also not sure I understand the role of the +linux-next and +master
> branches in linux-ubifs.git. Typically 'next' means for the current+1
> release (i.e., 4.1), while 'not-next' (i.e., your master branch?) would
> be for the current release (4.0). But you have +master based on top of
> +linux-next.

I'm using Artem's scheme. next is the branch Linus pulls from.
Artem, why are the two UBIFS fixes from master not in next?
I thought you want to send a pull request to Linus?
If you want, I can do that. I'm anyway preparing some UBI fixes.

Thanks,
//richard
Artem Bityutskiy March 17, 2015, 9:08 a.m. UTC | #4
On Mon, 2015-03-16 at 19:21 +0100, Richard Weinberger wrote:
> Hi!
> 
> Am 16.03.2015 um 19:08 schrieb Brian Norris:
> > On Mon, Feb 23, 2015 at 10:21:53PM +0100, Richard Weinberger wrote:
> >> Am 23.02.2015 um 22:07 schrieb Brian Norris:
> >>> commit 0e707ae79ba3 ("UBI: do propagate positive error codes up") seems
> >>> to have produced an unintended change in the control flow here.
> >>>
> >>> Completely untested, but it looks obvious.
> >>>
> >>> Caught by Coverity, which didn't like the indentation. CID 1271184.
> >>>
> >>> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> >>> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> >>> ---
> >>> Should go into 4.0, I expect.
> >>
> >> Good catch, patch tested and applied!
> > 
> > Is this going in 4.0? It fixes a typo in a hastily-applied patch that
> > made it to 4.0-rc1.
> 
> That's the plan.
> 
> > I'm also not sure I understand the role of the +linux-next and +master
> > branches in linux-ubifs.git. Typically 'next' means for the current+1
> > release (i.e., 4.1), while 'not-next' (i.e., your master branch?) would
> > be for the current release (4.0). But you have +master based on top of
> > +linux-next.
> 
> I'm using Artem's scheme. next is the branch Linus pulls from.
> Artem, why are the two UBIFS fixes from master not in next?

Just wanted to keep them there for several days.

> I thought you want to send a pull request to Linus?

No, I was not going to send this for 4.0. There is one fix which looks
important, but on the other hand, I did not test it, and no one reported
this problem and said this patch helps, so I was not going to send it
earlier.

> If you want, I can do that. I'm anyway preparing some UBI fixes.

Yes, please, merge master to linux-next.
diff mbox

Patch

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index da4c79259f67..16e34b37d134 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -425,9 +425,10 @@  retry:
 					ubi_warn(ubi, "corrupted VID header at PEB %d, LEB %d:%d",
 						 pnum, vol_id, lnum);
 					err = -EBADMSG;
-				} else
+				} else {
 					err = -EINVAL;
 					ubi_ro_mode(ubi);
+				}
 			}
 			goto out_free;
 		} else if (err == UBI_IO_BITFLIPS)