extrakeys: fix pubkey_sort_cmp test #302
Conversation
Instead of providing CTX directly, pass a cmp_data object containing CTX. Otherwise, memory sanitizer fails with "use-of-uninitialized-value".
| secp256k1_pubkey_sort(CTX, pk_ptr, 5); | ||
| for (j = 1; j < 5; j++) { | ||
| CHECK(secp256k1_pubkey_sort_cmp(&pk_ptr[j - 1], &pk_ptr[j], CTX) <= 0); | ||
| secp256k1_pubkey_sort_cmp_data cmp_data; |
There was a problem hiding this comment.
I confirmed that declaring the cmp_data variable at the beginning of the function does not cause any issues. This means we do not need to create a new instance for each iteration.
I reviewed the secp256k1_pubkey_sort_cmp function, and it appears that the ctx variable is not utilized within the secp256k1_pubkey_load function. My understanding of this usage is limited, so please correct me if I am mistaken.
There was a problem hiding this comment.
It's correct that ctx is never used (which is why the old, incorrect code could pass the tests).
As for scoping, IMO we should keep cmp_data as tightly scoped as we can, because it makes the code easier to read.
|
I'm gonna go ahead and merge this -- I can't imagine the CI failures are the fault of this PR. |
4 participants
Instead of providing CTX directly, pass a cmp_data object containing CTX. Otherwise, memory sanitizer fails with "use-of-uninitialized-value".