Fix the easy -Wformat-signedness errors.
GCC has a warning that complains about even more type mismatches in
printf. Some of these are a bit messy and will be fixed in separate CLs.
This covers the easy ones.
The .*s stuff is unfortunate, but printf has no size_t-clean string
printer. ALPN protocol lengths are bound by uint8_t, so it doesn't
really matter.
The IPv6 printing one is obnoxious and arguably a false positive. It's
really a C language flaw: all types smaller than int get converted to
int when you do arithmetic. So something like this first doesn't
overflow the shift because it computes over int, but then the result
overall is stored as an int.
uint8_t a, b;
(a << 8) | b
On the one hand, this fixes a "missing" cast to uint16_t before the
shift. At the same time, the incorrect final type means passing it to
%x, which expects unsigned int. The compiler has forgotten this value
actually fits in uint16_t and flags a warning. Mitigate this by storing
in a uint16_t first.
The story doesn't quite end here. Arguments passed to variadic functions
go through integer promotion[0], so the argument is still passed to
snprintf as an int! But then va_arg allows for a signedness mismatch[1],
provided the value is representable in both types. The combination means
that %x, though actually paired with unsigned, also accept uint8_t and
uint16_t, because those are guaranteed to promote to an int that meets
[1]. GCC recognizes [1] applies here.
(There's also PRI16x, but that's a bit tedious to use and, in glibc, is
defined as plain "x" anyway.)
[0] https://en.cppreference.com/w/c/language/conversion#Default_argument_promotions
[1] https://en.cppreference.com/w/c/variadic/va_arg
Bug: 450
Change-Id: Ic1d41356755a18ab922956dd2e07b560470341f4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50765
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/crypto/dsa/dsa_test.cc b/crypto/dsa/dsa_test.cc
index 4682131..cee783d 100644
--- a/crypto/dsa/dsa_test.cc
+++ b/crypto/dsa/dsa_test.cc
@@ -243,7 +243,7 @@
fprintf(out, "%02X%02X%02X%02X ", seed[i], seed[i + 1], seed[i + 2],
seed[i + 3]);
}
- fprintf(out, "\ncounter=%d h=%ld\n", counter, h);
+ fprintf(out, "\ncounter=%d h=%lu\n", counter, h);
if (counter != 105) {
fprintf(stderr, "counter should be 105\n");
diff --git a/crypto/evp/print.c b/crypto/evp/print.c
index 3621d5f..0f4b65e 100644
--- a/crypto/evp/print.c
+++ b/crypto/evp/print.c
@@ -253,7 +253,7 @@
if (priv_key) {
if (!BIO_indent(bp, off, 128) ||
- BIO_printf(bp, "%s: (%d bit)\n", ktype, BN_num_bits(x->p)) <= 0) {
+ BIO_printf(bp, "%s: (%u bit)\n", ktype, BN_num_bits(x->p)) <= 0) {
goto err;
}
}
@@ -368,7 +368,7 @@
}
order = BN_new();
if (order == NULL || !EC_GROUP_get_order(group, order, NULL) ||
- BIO_printf(bp, "%s: (%d bit)\n", ecstr, BN_num_bits(order)) <= 0) {
+ BIO_printf(bp, "%s: (%u bit)\n", ecstr, BN_num_bits(order)) <= 0) {
goto err;
}
diff --git a/crypto/fipsmodule/rand/urandom_test.cc b/crypto/fipsmodule/rand/urandom_test.cc
index b9e9007..0a1d753 100644
--- a/crypto/fipsmodule/rand/urandom_test.cc
+++ b/crypto/fipsmodule/rand/urandom_test.cc
@@ -91,7 +91,7 @@
switch (type) {
case Syscall::kGetRandom:
- snprintf(buf, sizeof(buf), "getrandom(_, %zu, %d)", length, flags);
+ snprintf(buf, sizeof(buf), "getrandom(_, %zu, %u)", length, flags);
break;
case Syscall::kOpen:
diff --git a/crypto/x509v3/v3_alt.c b/crypto/x509v3/v3_alt.c
index ce1c6e5..eb9c975 100644
--- a/crypto/x509v3/v3_alt.c
+++ b/crypto/x509v3/v3_alt.c
@@ -172,12 +172,13 @@
case GEN_IPADD:
p = gen->d.ip->data;
if (gen->d.ip->length == 4)
- BIO_snprintf(oline, sizeof oline,
+ BIO_snprintf(oline, sizeof(oline),
"%d.%d.%d.%d", p[0], p[1], p[2], p[3]);
else if (gen->d.ip->length == 16) {
oline[0] = 0;
for (i = 0; i < 8; i++) {
- BIO_snprintf(htmp, sizeof htmp, "%X", p[0] << 8 | p[1]);
+ uint16_t v = ((uint16_t)p[0] << 8) | p[1];
+ BIO_snprintf(htmp, sizeof(htmp), "%X", v);
p += 2;
OPENSSL_strlcat(oline, htmp, sizeof(oline));
if (i != 7)
@@ -246,7 +247,8 @@
else if (gen->d.ip->length == 16) {
BIO_printf(out, "IP Address");
for (i = 0; i < 8; i++) {
- BIO_printf(out, ":%X", p[0] << 8 | p[1]);
+ uint16_t v = ((uint16_t)p[0] << 8) | p[1];
+ BIO_printf(out, ":%X", v);
p += 2;
}
BIO_puts(out, "\n");
diff --git a/crypto/x509v3/v3_ncons.c b/crypto/x509v3/v3_ncons.c
index 739a59e..f58873e 100644
--- a/crypto/x509v3/v3_ncons.c
+++ b/crypto/x509v3/v3_ncons.c
@@ -203,7 +203,8 @@
p[0], p[1], p[2], p[3], p[4], p[5], p[6], p[7]);
} else if (len == 32) {
for (i = 0; i < 16; i++) {
- BIO_printf(bp, "%X", p[0] << 8 | p[1]);
+ uint16_t v = ((uint16_t)p[0] << 8) | p[1];
+ BIO_printf(bp, "%X", v);
p += 2;
if (i == 7)
BIO_puts(bp, "/");
diff --git a/ssl/ssl_privkey.cc b/ssl/ssl_privkey.cc
index 8462ebf..41a13a2 100644
--- a/ssl/ssl_privkey.cc
+++ b/ssl/ssl_privkey.cc
@@ -663,7 +663,7 @@
// Note that the loop runs to len+1, i.e. it'll process the terminating NUL.
for (size_t offset = 0; offset < len+1; offset++) {
- const char c = str[offset];
+ const unsigned char c = str[offset];
switch (c) {
case '+':
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index d0bd680..3286081 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -4694,7 +4694,7 @@
}
}
char retry_count[256];
- snprintf(retry_count, sizeof(retry_count), "%d", std::get<1>(params.param));
+ snprintf(retry_count, sizeof(retry_count), "%u", std::get<1>(params.param));
ret += "_";
ret += retry_count;
ret += "Retries_";
diff --git a/ssl/test/handshake_util.cc b/ssl/test/handshake_util.cc
index b999831..6516547 100644
--- a/ssl/test/handshake_util.cc
+++ b/ssl/test/handshake_util.cc
@@ -255,7 +255,7 @@
return false;
}
if (bytes_written != bytes_read) {
- fprintf(stderr, "short write (%zu of %d bytes)\n", bytes_written,
+ fprintf(stderr, "short write (%zd of %d bytes)\n", bytes_written,
bytes_read);
return false;
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 7d1cefa..16cb971 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -1047,17 +1047,16 @@
const size_t num_received = sk_X509_NAME_num(received);
if (num_received != num_expected) {
- fprintf(stderr, "expected %u names in CertificateRequest but got %u.\n",
- static_cast<unsigned>(num_expected),
- static_cast<unsigned>(num_received));
+ fprintf(stderr, "expected %zu names in CertificateRequest but got %zu.\n",
+ num_expected, num_received);
return false;
}
for (size_t i = 0; i < num_received; i++) {
if (X509_NAME_cmp(sk_X509_NAME_value(received, i),
sk_X509_NAME_value(expected.get(), i)) != 0) {
- fprintf(stderr, "names in CertificateRequest differ at index #%d.\n",
- static_cast<unsigned>(i));
+ fprintf(stderr, "names in CertificateRequest differ at index #%zu.\n",
+ i);
return false;
}
}
diff --git a/tool/transport_common.cc b/tool/transport_common.cc
index cba3c7b..991c808 100644
--- a/tool/transport_common.cc
+++ b/tool/transport_common.cc
@@ -306,13 +306,13 @@
const uint8_t *next_proto;
unsigned next_proto_len;
SSL_get0_next_proto_negotiated(ssl, &next_proto, &next_proto_len);
- BIO_printf(bio, " Next protocol negotiated: %.*s\n", next_proto_len,
- next_proto);
+ BIO_printf(bio, " Next protocol negotiated: %.*s\n",
+ static_cast<int>(next_proto_len), next_proto);
const uint8_t *alpn;
unsigned alpn_len;
SSL_get0_alpn_selected(ssl, &alpn, &alpn_len);
- BIO_printf(bio, " ALPN protocol: %.*s\n", alpn_len, alpn);
+ BIO_printf(bio, " ALPN protocol: %.*s\n", static_cast<int>(alpn_len), alpn);
const char *host_name = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
if (host_name != nullptr && SSL_is_server(ssl)) {
