Dangling 'else' in ecp.c

[irwir]

https://github.com/ARMmbed/mbedtls/blob/8266acacc8d6e1c65fba9a048f56339d0827b2fe/library/ecp.c#L1895

Static analysers might find this line suspicious:

• else after return statement is redundant
• an empty line after else is possibly a missing or forgotten statement

The suggestion is to delete the line.

[RonEld]

@irwir thank you for reporting this!
This isn't a dangling else statement, as the compiler will regard this as an else for the if statement, and run the next statement ( in line 1897 ) as the else. So deleting this line will result in a different functionality. ( perhaps the intended one )
I agree this is not readable, and might not what the author intended.
We will look into this.

[ciarmcom]

ARM Internal Ref: IOTSSL-2372

[irwir]

@RonEld

This isn't a dangling else statement, as the compiler will regard this as an else for the if statement

Commonly used styles do not add empty lines after else keyword.
It could be that a programmer reserved a place for a statement, but got distracted and forgot to write a statement.
In that sense it is "dangling", and this is the reason to be flagged in static analysis.

So deleting this line will result in a different functionality.

Could you give an example when deleting this else would change functionality?

[RonEld]

@irwir

Commonly used styles do not add empty lines after else keyword.

I agree with you that this is not good style, And this else keyword is definitely suspicious, and needs to be checked. If a static analyzer shouted on this, it definitely should be fixed. It isn't a dangling else statement, in the sense that the compiler will not ignore it, as the compiler will ignore the extra line, and consider this as else for previous if statement.

Could you give an example when deleting this else would change functionality?

Current behavior is:

if A
{}
else if B
{}

You are suggesting to change it to:

if A
{}
if B
{}

But I see your point, because if A is true, then we return and exit the function, so in this specific use case, there is no real change in the functionality with your suggestion.

[irwir]

But I see your point, because if A is true, then we return and exit the function, so in this specific use case, there is no rel change in the functionality with your suggestion.

Yes.
If linear execution order is always broken in the block after if A, then the following else is redundant.
Presence of that else becomes strictly a matter of style.