Wallet: Resolve compiler warnings in ec_privkey_import_der #12286

pull tamasblummer wants to merge 1 commits into bitcoin:master from tamasblummer:master changing 1 files +3 −3

tamasblummer commented at 2:16 PM on January 28, 2018: contributor

given: size_t x; char *end, *privkey; replace: if (end - privkey < x) with if (end < privkey || (size_t)(end - privkey) < x)

This eliminates below warnings if built with gcc 5.4.0:

key.cpp: In function ‘int ec_privkey_import_der(const secp256k1_context, unsigned char, const unsigned char*, size_t)’: key.cpp:51:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (end - privkey < lenb) { ^ key.cpp:57:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (end - privkey < len) { ^ key.cpp:71:37: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (oslen > 32 || end - privkey < oslen) {
resolve compiler warnings for comparison of signed with unsigned int 8c7ef6e56e
fanquake added the label Wallet on Jan 28, 2018
practicalswift commented at 4:14 PM on January 28, 2018: contributor

Related: #11577
laanwj commented at 1:24 PM on January 29, 2018: member

utACK 8c7ef6e
superac22 approved
devrandom commented at 6:26 PM on February 3, 2018: none

how about if (end < privkey + x) instead? seems easier to read and more concise.
devrandom commented at 6:29 PM on February 3, 2018: none

BTW, I notice that this function doesn't seem to be directly tested, but probably something for a further PR.
tamasblummer commented at 6:44 PM on February 3, 2018: contributor

@devrandom you are right, that is a more concise fix for the warning.
laanwj commented at 2:06 PM on February 8, 2018: member

#12351 has yet another solution: it just changes the type of the variables and doesn't touch the expressions at all. Maybe that's preferable?
devrandom commented at 4:44 PM on February 8, 2018: none

#12351 is at least as good of a solution, and might as well go with it since it's written
MarcoFalke commented at 9:57 PM on February 8, 2018: member

I'd prefer having only one pull request open. Can we move the review and discussion over to #12351, please?
MarcoFalke closed this on Feb 8, 2018
DrahtBot locked this on Sep 8, 2021

Contributors

Labels