Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...)

[practicalswift]

Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...):

In file included from src/secp256k1.c:15:0,
                 from src/tests.c:17:
src/ecmult_const_impl.h: In function ‘secp256k1_wnaf_const’:
src/ecmult_const_impl.h:117:20: warning: ‘u’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     wnaf[word] = u * global_sign;
                    ^

Note to reviewers: Perhaps an assert(…); is a bit drastic. What would be a more graceful way to handle this? :-)

[sipa]

We don't use assert in the secp256k1 codebase. Instead I suggest you:

• Turn the while loop into a do loop.
• Add a VERIFY_CHECK(size > 0); (which only affects the code in unit tests).

[practicalswift]

@sipa Makes perfect sense. Thanks for the quick feedback!

I've now pushed an updated version. Please re-review :-)

[Empact]

utACK 5f10203

[gmaxwell]

ACK

[ghost]

; ) 確認

[jonasnick]

utACK

[gmaxwell]

On re-review, I believe we need to VERIFY_CHECK w>0 for the loop transformation to be correct. (obviously w>0 or the code is already broken... so.) @sipa opinion?

[real-or-random]

I think it's best to just VERIFY_CHECK(w > 0) too. If w == 0, we even divide by 0 in the VERIFY_CHECK here:

secp256k1/src/ecmult_const_impl.h

Line 119 in 5f10203

VERIFY_CHECK(word == WNAF_SIZE_BITS(size, w));

[real-or-random]

@practicalswift: Could you update this?

[practicalswift]

@real-or-random Sure!

Just to clarify (better safe than sorry!):

Is the suggestion to change from:

VERIFY_CHECK(size > 0);

To:

VERIFY_CHECK(w > 0);
VERIFY_CHECK(size > 0);

And otherwise leave the PR unchanged?

[real-or-random]

Yes that's my suggestion. :)

[practicalswift]

@real-or-random Updated. Please re-review :-)

[real-or-random]

utACK 248f046

[gmaxwell]

ACK