Type conversion fixes #7918
Merged
Type conversion fixes #7918
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SparkiDev
force-pushed
the
type_conversion_fixes_3
branch
from
dddcc22 to
910740b
Compare
|
retest this please |
douzzer
requested changes
Aug 31, 2024
There was a problem hiding this comment.
Good amount of progress here, though of course it would be nicer if we just had self-consistent types in the first place, rather than an armada of casts to support legacy code and API signatures.
checkpoint for the record: on a default build of this PR, I count 150 remaining -Wconversions in the TLS layer, 39 in api.c, and 28 in the crypto layer, all but 2 in test.c. (and I've noted those 2 in my review.)
wolfssl/wolfcrypt/asn.h
Outdated
| @@ -670,7 +670,7 @@ WOLFSSL_LOCAL void SetASN_OID(ASNSetData *dataASN, int oid, int oidType); | |||
| do { \ | |||
| int ii; \ | |||
| for (ii = (node) + 1; ii < (int)(dataASNLen); ii++) { \ | |||
| if ((asn)[ii].depth <= (asn)[node].depth) \ | |||
| if ((asn)[ii].depth <= (asn)[(node)].depth) \ | |||
There was a problem hiding this comment.
[(node)] not needed -- parens and brackets have the same (highest) associativity, so if [node] is broken, so is [(node)] -- but of course [node] is fine.
wolfssl/wolfcrypt/asn.h
Outdated
| @@ -689,7 +689,7 @@ WOLFSSL_LOCAL void SetASN_OID(ASNSetData *dataASN, int oid, int oidType); | |||
| int ii; \ | |||
| (dataASN)[node].noOut = 1; \ | |||
| for (ii = (node) + 1; ii < (int)(dataASNLen); ii++) { \ | |||
| if ((asn)[ii].depth <= (asn)[node].depth) \ | |||
| if ((asn)[ii].depth <= (asn)[(node)].depth) \ | |||
There was a problem hiding this comment.
ditto -- and note that if [(node)] were needed here, it would also be needed a couple lines up (currently L691).
wolfssl/wolfcrypt/asn.h
Outdated
| @@ -594,7 +594,7 @@ WOLFSSL_LOCAL void SetASN_OID(ASNSetData *dataASN, int oid, int oidType); | |||
| * @param [in] oidType Type of OID. | |||
| */ | |||
| #define SetASN_OID(dataASN, oid, oidType) \ | |||
| (dataASN)->data.buffer.data = OidFromId(oid, oidType, \ | |||
| (dataASN)->data.buffer.data = OidFromId((oid), (oidType), \ | |||
There was a problem hiding this comment.
parens not needed around values separated by commas -- commas have uniquely minimal associativity.
wolfcrypt/src/pkcs7.c
Outdated
| @@ -819,7 +819,7 @@ int wc_PKCS7_Init(PKCS7* pkcs7, void* heap, int devId) | |||
|
|
|||
| isDynamic = pkcs7->isDynamic; | |||
| XMEMSET(pkcs7, 0, sizeof(PKCS7)); | |||
| pkcs7->isDynamic = isDynamic; | |||
| pkcs7->isDynamic = (isDynamic == 1); | |||
There was a problem hiding this comment.
pedantically & safer, the right RHS construct here is isDynamic != 0.
wolfcrypt/src/pkcs7.c
Outdated
| @@ -3428,7 +3436,7 @@ int wc_PKCS7_SetDetached(PKCS7* pkcs7, word16 flag) | |||
| if (pkcs7 == NULL || (flag != 0 && flag != 1)) | |||
| return BAD_FUNC_ARG; | |||
|
|
|||
| pkcs7->detached = flag; | |||
| pkcs7->detached = (flag == 1); | |||
wolfcrypt/src/pkcs7.c
Outdated
| } | ||
|
|
||
| /* Free pkcs7 resources but not the structure itself */ | ||
| pkcs7->isDynamic = 0; | ||
| wc_PKCS7_Free(pkcs7); | ||
| pkcs7->isDynamic = isDynamic; | ||
| pkcs7->isDynamic = (isDynamic == 1); |
wolfcrypt/src/pkcs7.c
Outdated
| @@ -6231,7 +6241,7 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, | |||
| } | |||
|
|
|||
| /* Restore content is PKCS#7 flag */ | |||
| pkcs7->contentIsPkcs7Type = contentIsPkcs7Type; | |||
| pkcs7->contentIsPkcs7Type = (contentIsPkcs7Type == 1); | |||
wolfcrypt/src/pkcs7.c
Outdated
| @@ -14351,7 +14393,7 @@ int wc_PKCS7_SetStreamMode(PKCS7* pkcs7, byte flag, | |||
| return BAD_FUNC_ARG; | |||
| } | |||
| #ifdef ASN_BER_TO_DER | |||
| pkcs7->encodeStream = flag; | |||
| pkcs7->encodeStream = (flag == 1); | |||
wolfcrypt/src/pkcs7.c
Outdated
| @@ -14387,7 +14429,7 @@ int wc_PKCS7_SetNoCerts(PKCS7* pkcs7, byte flag) | |||
| if (pkcs7 == NULL) { | |||
| return BAD_FUNC_ARG; | |||
| } | |||
| pkcs7->noCerts = flag; | |||
| pkcs7->noCerts = (flag == 1); | |||
wolfcrypt/src/pkcs7.c
Outdated
| @@ -8194,13 +8207,15 @@ static int wc_PKCS7_EncryptContent(PKCS7* pkcs7, int encryptOID, byte* key, | |||
| ret = wc_AesGcmEncrypt(aes, out, in, inSz, iv, ivSz, | |||
There was a problem hiding this comment.
I'm seeing
wolfcrypt/src/pkcs7.c:8207:62: error: conversion to 'word32' {aka 'unsigned int'} from 'int' may change the sign of the result [-Werror=sign-conversion]
8207 | ret = wc_AesGcmEncrypt(aes, out, in, inSz, iv, ivSz,
| ^~~~
wolfcrypt/src/pkcs7.c:8207:72: error: conversion to 'word32' {aka 'unsigned int'} from 'int' may change the sign of the result [-Werror=sign-conversion]
8207 | ret = wc_AesGcmEncrypt(aes, out, in, inSz, iv, ivSz,
| ^~~~
Changes to get compilation with -Wconversion passing on the files.
SparkiDev
force-pushed
the
type_conversion_fixes_3
branch
from
910740b to
ed7beb4
Compare
|
retest this please |
douzzer
approved these changes
Sep 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Changes to get compilation with -Wconversion passing on the files.
Testing
./configure --disable-shared --enable-all CFLAGS=-Wconversion
Compile the files changed only.
Checklist