Gregory Maxwell [Fri, 27 Mar 2015 23:14:17 +0000 (23:14 +0000)]
Brace all the if/for/while.
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.
Pieter Wuille [Fri, 27 Mar 2015 20:49:45 +0000 (13:49 -0700)]
Merge pull request #229
efc571c Add simple testcases for signing with rfc6979 extra entropy. (Gregory Maxwell) 1573a10 Add ability to pass extra entropy to rfc6979 (Pieter Wuille)
Gregory Maxwell [Tue, 17 Feb 2015 09:01:48 +0000 (01:01 -0800)]
Eliminate multiple-returns from secp256k1.c.
Goto, multiple returns, continue, and/or multiple breaks in a
loop are often used to build complex or non-local control
flow in software.
(They're all basically the same thing, and anyone axiomatically
opposing goto and not the rest is probably cargo-culting from
the title of Dijkstra's essay without thinking hard about it.)
Personally, I think the current use of these constructs in the
code base is fine: no where are we using them to create control-
flow that couldn't easily be described in plain English, which
is hard to read or reason about, or which looks like a trap for
future developers.
Some, however, prefer a more rules based approach to software
quality. In particular, MISRA forbids all of these constructs,
and for good experience based reasons. Rules also have the
benefit of being machine checkable and surviving individual
developers.
(To be fair-- MISRA also has a process for accommodating code that
breaks the rules for good reason).
I think that in general we should also try to satisfy the rules-
based measures of software quality, except where there is an
objective reason not do: a measurable performance difference,
logic that turns to spaghetti, etc.
Changing out all the multiple returns in secp256k1.c appears to
be basically neutral: Some parts become slightly less clear,
some parts slightly more.
Pieter Wuille [Fri, 13 Feb 2015 00:26:40 +0000 (16:26 -0800)]
Merge pull request #206
34b898d Additional comments for the testing PRNG and a seeding fix. (Gregory Maxwell) 6efd6e7 Some comments explaining some of the constants in the code. (Gregory Maxwell)
Pieter Wuille [Sun, 25 Jan 2015 13:13:31 +0000 (09:13 -0400)]
Merge pull request #199
fcc48c4 Remove the non-storage cmov (Pieter Wuille) 55422b6 Switch ecmult_gen to use storage types (Pieter Wuille) 41f8455 Use group element storage type in EC multiplications (Pieter Wuille) e68d720 Add group element storage type (Pieter Wuille) ff889f7 Field storage type (Pieter Wuille)
Gregory Maxwell [Fri, 23 Jan 2015 05:48:27 +0000 (05:48 +0000)]
Convert field code to strict C89 (+ long long, +__int128)
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.
Pieter Wuille [Sun, 4 Jan 2015 14:23:03 +0000 (15:23 +0100)]
Merge pull request #177
7688e34 Add magnitude limits to secp256k1_fe_verify to ensure that it's own tests function correctly. (Gregory Maxwell) 70ae0d2 Use secp256k1_fe_equal_var in secp256k1_fe_sqrt_var. (Gregory Maxwell)
Gregory Maxwell [Wed, 31 Dec 2014 13:56:00 +0000 (05:56 -0800)]
Use secp256k1_fe_equal_var in secp256k1_fe_sqrt_var.
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.
Pieter Wuille [Tue, 23 Dec 2014 13:38:15 +0000 (14:38 +0100)]
Merge pull request #163
bbd5ba7 Use rfc6979 as default nonce generation function (Pieter Wuille) b37fbc2 Implement SHA256 / HMAC-SHA256 / RFC6979. (Pieter Wuille) c6e7f4e [API BREAK] Use a nonce-generation function instead of a nonce (Pieter Wuille)