Unbraced statements spanning multiple lines has been shown in many
projects to contribute to the introduction of bugs and a failure
to catch them in review, especially for maintenance on infrequently
modified code.
Most, but not all, of the existing practice in the codebase were not
cases that I would have expected to eventually result in bugs but
applying it as a rule makes it easier for other people to safely
contribute.
I'm not aware of any such evidence for the case with the statement
on a single line, but some people strongly prefer to never do that
and the opposite rule of "_always_ use a single line for single
statement blocks" isn't a reasonable rule for formatting reasons.
Might as well brace all these too, since that's more universally
acceptable.
[In any case, I seem to have introduced the vast majority of the
single-line form (as they're my preference where they fit).]
This also removes a broken test which is no longer needed.
This makes the software more portable to embedded systems
and static analysis tools.
Sadly, it can't result in identical binaries because C99 mixed
declarations seem to make GCC emit superfluous stack-pointer
updates. The compiler is also somewhat dependent on the
declaration order.
In theory this should be faster, since secp256k1_fe_equal_var is able to
shortcut the normalization. On x86_64 the improvement appears to be in
the noise for me. At least it makes the code cleaner.
- secp256k1_fe_sqrt now checks that the value it calculated is actually a square root.
- Add return values to secp256k1_fe_sqrt and secp256k1_ge_set_xo.
- Callers of secp256k1_ge_set_xo can use return value instead of explicit validity checks
- Add random value tests for secp256k1_fe_sqrt
