GH1383: Add casts to ERR_PACK #1385
Conversation
| (((unsigned int)(l) & 0x0FF) << 24L) | \ | ||
| (((unsigned int)(f) & 0xFFF) << 12L) | \ | ||
| (((unsigned int)(r) & 0xFFF) ) ) | ||
| # define ERR_GET_LIB(l) (int)((((unsigned long)l) >> 24L) & 0x0FFL) |
There was a problem hiding this comment.
You might as well cast these to unsigned int as well for consistency. Does ERR_GET_REASON also need a cast then?
There was a problem hiding this comment.
you mean liens 140,141,142? If so yes, that makes sense.
There was a problem hiding this comment.
Assuming I got it right, I pushed an additional commit. Please take a look?
There was a problem hiding this comment.
Um, I meant unifying all casts on the input to silence the warnings:
a) unsigned long -> unsigned int; and
b) a cast on the input of ERR_GET_REASON, similarly to the others.
We shouldn't change the return type.
There was a problem hiding this comment.
Doh! Fixed. Revised commit pushed.
There was a problem hiding this comment.
Hmm, I looked at the docs meanwhile - ERR_PACK is documented to take in ints, so these three casts are needed. But the ERR_GET_ flavours take an unsigned long so actually I think these input casts should simply be removed.
(Sorry for making you go around in circles.)
There was a problem hiding this comment.
No prob. I like getting dizzy :) updated commit pushed.
|
+1 but see comment |
And update script to not re-introduce whitespace glitch.
Reviewed-by: Emilia Käsper <emilia@openssl.org> (Merged from #1385
And update script to not re-introduce whitespace glitch.