Fix current CI failures on push #21823
Conversation
The TLS record type is a single byte value so we can use uint8_t for it. This allows passing its address directly to SSL_trace() instead of converting it to a single byte type first.
They produce a warning `suggest braces around initialization of subobject` otherwise.
t8m
added
branch: master
|
The last commit that adjusts the CI runs is to be dropped when merging. |
|
I am skeptical about the value of |
Yeah, I hate it as well. We could remove it from the strict warning set. |
Add -Wno-missing-braces to silence old clang compilers And drop unnecessary braces in zeroing initializers.
t8m
force-pushed
the
cross-comp-failure
branch
from
290d15e
to
e7ae43e
Compare
|
Instead of adding the braces I've switched-off the warning. |
t8m
force-pushed
the
cross-comp-failure
branch
from
f07a8e6
to
e7ae43e
Compare
| @@ -507,7 +507,7 @@ struct ossl_record_method_st { | |||
| * multiple records in one go and buffer them. | |||
| */ | |||
| int (*read_record)(OSSL_RECORD_LAYER *rl, void **rechandle, int *rversion, | |||
| int *type, unsigned char **data, size_t *datalen, | |||
| uint8_t *type, unsigned char **data, size_t *datalen, | |||
There was a problem hiding this comment.
If we're going to change this type I think it has ripple effects in libssl, e.g. I would also suggest changing the type of type and recvd_type in ssl3_read_bytes and dtls1_read_bytes
There was a problem hiding this comment.
AFAIK these are assigned and not passed as pointer to them so type conversion should happen fine. But I can change it anyway if you prefer that.
There was a problem hiding this comment.
@mattcaswell it was quite spread throughout the code but AFAIK done now, please look.
There was a problem hiding this comment.
it was quite spread throughout the code but AFAIK done now, please look.
Yeah - this is what I meant by a ripple effect.
But I can change it anyway if you prefer that.
I really dislike this type dissonance where we use different types for the same thing in different places. Looks much better now.
mattcaswell
removed
the
approval: otc review pending
hlandau
added
approval: done
openssl-machine
removed
the
approval: done
|
This pull request is ready to merge |
openssl-machine
added
the
approval: ready to merge
|
Merged to master. Thank you. |
The TLS record type is a single byte value so we can use uint8_t for it. This allows passing its address directly to SSL_trace() instead of converting it to a single byte type first. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #21823)
Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #21823)
They produce a warning `suggest braces around initialization of subobject` otherwise. Add -Wno-missing-braces to silence old clang compilers And drop unnecessary braces in zeroing initializers. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #21823)
Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #21823)
Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #21823)
No description provided.