Change in ...flashrom[master]: atavia.c: Remove unneeded 'else'

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/33343

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

atavia.c: Remove unneeded 'else'

'else' is not needed after a 'break' or 'return'.

Change-Id: Ib43a8c186c34c85f964df648564fa7f5aef10412
Signed-off-by: Elyes HAOUAS 
---
M atavia.c
1 file changed, 2 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/33343/1

diff --git a/atavia.c b/atavia.c
index 80bba58..3b4a5bd 100644
--- a/atavia.c
+++ b/atavia.c
@@ -102,10 +102,9 @@
    	if (((access & BROM_TRIGGER) == 0) && (status & BROM_ERROR_STATUS) == 0) {
    		ready = true;
    		break;
-		} else {
-			programmer_delay(1);
-			continue;
    	}
+		programmer_delay(1);
+		continue;
    }

msg_pdbg2("\n%s: %s after %d tries (access=0x%02x, status=0x%02x)\n",

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Patch Set 1: Code-Review+2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Patch Set 1:

(1 comment)

Can we discuss this first? How is not having the `else`
better? The `continue` on the else path also wasn't
necessary, but I assume the author put it there to
balance the branches, for better legibility.

https://review.coreboot.org/#/c/33343/1/atavia.c 
File atavia.c:

https://review.coreboot.org/#/c/33343/1/atavia.c@107 
PS1, Line 107: 		continue;
This is... well, looks just wrong.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Patch Set 1: Code-Review+1

(1 comment)
> Patch Set 1:
>
> (1 comment)
>
> Can we discuss this first? How is not having the `else`
> better? The `continue` on the else path also wasn't
> necessary, but I assume the author put it there to
> balance the branches, for better legibility.

I've just checked that the resulting code is the same, functionally speaking. But yes, this one is ugly.

https://review.coreboot.org/#/c/33343/1/atavia.c 
File atavia.c:

https://review.coreboot.org/#/c/33343/1/atavia.c@107 
PS1, Line 107: 		continue;
> This is... well, looks just wrong.

Indeed.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Patch Set 1:

(1 comment)

Indeed, it is more "conformable" to have the 'else' branch here.

https://review.coreboot.org/#/c/33343/1/atavia.c 
File atavia.c:

https://review.coreboot.org/#/c/33343/1/atavia.c@107 
PS1, Line 107: 		continue;
> Indeed.

for my understanding, there is no difference between old and new code.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/33343/1/atavia.c 
File atavia.c:

https://review.coreboot.org/#/c/33343/1/atavia.c@107 
PS1, Line 107: 		continue;
> for my understanding, there is no difference between old and new code.

Yes, it does exactly the same (hence my initial +2) but the "continue" at the end is just silly (hence my current +1)

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Patch Set 1:

(1 comment)
> Indeed, it is more "conformable" to have the 'else' branch here.

What I'm concerned about is if we can argue that the code *is
generally better* with the else removed (that `continue` aside)
and make that a rule. Without a rule, somebody could just push
another patch that adds the else back...

In other words, "what does this fix?".

Actually, I agree to the removal of `else` in all three patches.
But without a rule...

Is there any compiler warning, or how did you spot these?

https://review.coreboot.org/#/c/33343/1/atavia.c 
File atavia.c:

https://review.coreboot.org/#/c/33343/1/atavia.c@107 
PS1, Line 107: 		continue;
> for my understanding, there is no difference between old and new code.

That's right. But an `continue` at the end of a loop body is
obviously a no-op.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Patch Set 1:

I agree.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/33343 )

Change subject: atavia.c: Remove unneeded 'else'
......................................................................

Abandoned