cmd/jwk: check return from jose_jwk_thp_buf() properly#171
cmd/jwk: check return from jose_jwk_thp_buf() properly#171sarroutbi merged 1 commit intolatchset:masterfrom
Conversation
51eac76 to
a18ced7
Compare
sarroutbi
left a comment
There was a problem hiding this comment.
I would follow an if/else if approach to ease code readibility. Something like:
size_t nbytes = jose_jwk_thp_buf(NULL, jwk, opt.hash, dec, sizeof(dec)); // dec is the output buffer
+ if (nbytes == SIZE_MAX) {
+ fprintf(stderr, "Error making thumbprint: Reached %ul (SIZE_MAX) nbytes\n", SIZE_MAX); // Please, not %ul might be adjusted here
+ return EXIT_FAILURE;
+ } else if (nbytes == 0) {
fprintf(stderr, "Error making thumbprint: nbytes equals 0.\n");
return EXIT_FAILURE;
}
This way, in case error takes place, it will be clear where it is taking place
It doesn't look like a big (if at all) improvement in this case. I will just check for the SIZE_MAX instead and make it simpler. |
a18ced7 to
066fc53
Compare
👍 OTOH ... is it feasible to add a unit test to cover this? I am just wondering |
|
Good point. Yeah, we can add the example from the issue report, I believe. Let me do that. |
jose_jwk_thp_buf() returns, on success, the number of bytes written; otherwise, it returns SIZE_MAX. We now check its return properly, from jcmd_jwk_thp(), which is used by the CLI utility. Signed-off-by: Sergio Correia <scorreia@redhat.com>
066fc53 to
e391364
Compare
sarroutbi
left a comment
There was a problem hiding this comment.
Thanks for including the unit test. LGTM
jose_jwk_thp_buf()returns, on success, the number of bytes written; otherwise, it returnsSIZE_MAX.We now check its return properly, from
jcmd_jwk_thp(), which is used by the CLI utility.Fixes: #170