Author: brane
Date: Sun Jul 6 09:52:17 2025
New Revision: 1926988
URL: http://svn.apache.org/viewvc?rev=1926988&view=rev
Log:
Fix some more narrowing, unreachable-code and shadowing warnings.
* CMakeLists.txt, SConstruct: Add -Wshadow to the default warning options.
== Narrowing conversions ==
* buckets/ssl_buckets.c
(bio_bucket_read): ### Safe cast apr_size_t -> int
(bio_file_read): ### Safe cast apr_size_t -> int
(bio_file_write): ### Safe cast apr_size_t -> int
(serf_ssl_negotiate_protocol): ### Safe cast apr_size_t -> int
(ocsp_callback): Make 'len' a long, which is the type of the return value
from SSL_get_tlsext_status_ocsp_resp().
(ssl_decrypt): Add an int 'ssl_bufsize' with a checked conversion from
the apr_size_t 'bufsize', whos type can not be changed because it's a
callback type parameter.
(ssl_encrypt): The 'interim_len' is initializef from an int, so it may
as well be an int, too.
(serf_ssl_ocsp_request_t): Change 'der_request_size' to an int. It is
initialized from an int and never changed, and used in contexts that
expect an int.
(serf_ssl_ocsp_request_imp): Change 'der_request_size' and 'der_id_size'
to int, for similar reasons. The apr_base64 code uses int sizes, not
apr_size_t sizes.
== Unreachable code ==
* buckets/ssl_buckets.c
(serf_ssl_set_hostname): Return APR_ENOTIMPL from an #else branch of the
#ifdef. While not strictly necessary, it avoids confusion.
(serf_ssl_check_cert_status_request): Likewise.
(serf_ssl_use_compression): Likewise. Before this change it returned
APR_EGENERAL, but APR_ENOTIMPL is more appropriate.
== Variable shadowing ==
* buckets/ssl_buckets.c
(ssl_need_client_cert): Rename 'c' to 'x509'. It was shadowed by another
'c' of the same type, at line 1706. This was not actually a bug, since
'x509' is only initialized after 'c' goes out of scope, but it's still
confusing. Also shows why one shouldn't write 1000-line functions.
* test/serf_get.c (main),
test/serf_httpd.c (main): Remove second declarations of 'status'.
Modified:
serf/trunk/CMakeLists.txt
serf/trunk/SConstruct
serf/trunk/buckets/ssl_buckets.c
serf/trunk/test/serf_get.c
serf/trunk/test/serf_httpd.c
Modified: serf/trunk/CMakeLists.txt
URL:
http://svn.apache.org/viewvc/serf/trunk/CMakeLists.txt?rev=1926988&r1=1926987&r2=1926988&view=diff
==============================================================================
--- serf/trunk/CMakeLists.txt (original)
+++ serf/trunk/CMakeLists.txt Sun Jul 6 09:52:17 2025
@@ -365,6 +365,7 @@ if(NOT MSVC)
list(APPEND SERF_C_WARNINGS "-Wall")
list(APPEND SERF_C_WARNINGS "-Wdeclaration-after-statement")
list(APPEND SERF_C_WARNINGS "-Wmissing-prototypes")
+ list(APPEND SERF_C_WARNINGS "-Wshadow")
string(APPEND CMAKE_C_FLAGS_DEBUG " -O0")
Modified: serf/trunk/SConstruct
URL:
http://svn.apache.org/viewvc/serf/trunk/SConstruct?rev=1926988&r1=1926987&r2=1926988&view=diff
==============================================================================
--- serf/trunk/SConstruct (original)
+++ serf/trunk/SConstruct Sun Jul 6 09:52:17 2025
@@ -361,6 +361,7 @@ if sys.platform != 'win32':
env.SerfAppendIf(['CFLAGS'], r'-(ansi|std=c\d+)', CFLAGS=['-std=c89'])
env.Append(CCFLAGS=['-Wdeclaration-after-statement',
'-Wmissing-prototypes',
+ '-Wshadow',
'-Wall'])
if debug:
Modified: serf/trunk/buckets/ssl_buckets.c
URL:
http://svn.apache.org/viewvc/serf/trunk/buckets/ssl_buckets.c?rev=1926988&r1=1926987&r2=1926988&view=diff
==============================================================================
--- serf/trunk/buckets/ssl_buckets.c (original)
+++ serf/trunk/buckets/ssl_buckets.c Sun Jul 6 09:52:17 2025
@@ -430,7 +430,9 @@ static int bio_bucket_read(BIO *bio, cha
"bio_bucket_read received %"APR_SIZE_T_FMT" bytes (%d)\n", len,
status);
memcpy(in, data, len);
- return len;
+
+ /* Safe cast: len <= inlen */
+ return (int)len;
}
/* Returns the amount written. */
@@ -476,7 +478,8 @@ static int bio_file_read(BIO *bio, char
if (APR_STATUS_IS_EOF(status)) {
return -1;
} else {
- return len;
+ /* Safe cast: len <= inlen */
+ return (int)len;
}
}
@@ -494,7 +497,8 @@ static int bio_file_write(BIO *bio, cons
nbytes = inl;
apr_file_write(file, in, &nbytes);
- return nbytes;
+ /* Safe cast: nbytes <= inlen */
+ return (int)nbytes;
}
static int bio_file_gets(BIO *bio, char *in, int inlen)
@@ -694,7 +698,7 @@ static int ocsp_callback(SSL *ssl, void
serf_ssl_context_t *ctx = (serf_ssl_context_t*)baton;
OCSP_RESPONSE *response;
const unsigned char *resp_der;
- int len;
+ long len;
int failures = 0;
int cert_valid = 0;
@@ -1154,7 +1158,7 @@ static apr_status_t ssl_decrypt(void *ba
{
serf_ssl_context_t *ctx = baton;
apr_status_t status;
- int ssl_len;
+ int ssl_len, ssl_bufsize;
if (ctx->fatal_err)
return ctx->fatal_err;
@@ -1171,8 +1175,9 @@ static apr_status_t ssl_decrypt(void *ba
}
}
+ SERF__POSITIVE_TO_INT(ssl_bufsize, apr_size_t, bufsize);
serf__log(LOGLVL_DEBUG, LOGCOMP_SSL, __FILE__, ctx->config,
- "ssl_decrypt: begin %" APR_SIZE_T_FMT "\n", bufsize);
+ "ssl_decrypt: begin %d\n", ssl_bufsize);
ctx->want_read = FALSE; /* Reading now */
ctx->crypt_status = APR_SUCCESS; /* Clear before calling SSL */
@@ -1184,7 +1189,7 @@ static apr_status_t ssl_decrypt(void *ba
Luckily we can assume that we are called from the databuffer
implementation */
/* Is there some data waiting to be read? */
- ssl_len = SSL_read(ctx->ssl, buf, bufsize);
+ ssl_len = SSL_read(ctx->ssl, buf, ssl_bufsize);
if (ssl_len < 0) {
*len = 0;
@@ -1218,7 +1223,7 @@ static apr_status_t ssl_decrypt(void *ba
*len = ssl_len;
status = ctx->crypt_status;
serf__log(LOGLVL_DEBUG, LOGCOMP_SSLMSG, __FILE__, ctx->config,
- "---\n%.*s\n-(%"APR_SIZE_T_FMT")-\n", (int)*len, buf, *len);
+ "---\n%.*s\n-(%"APR_SIZE_T_FMT")-\n", ssl_len, buf, *len);
}
@@ -1315,7 +1320,7 @@ static apr_status_t ssl_encrypt(void *ba
/* Oh well, read from our stream now. */
interim_bufsize = bufsize;
do {
- apr_size_t interim_len;
+ int interim_len;
if (!ctx->want_read) {
struct iovec vecs[SERF__STD_IOV_COUNT];
@@ -1351,7 +1356,7 @@ static apr_status_t ssl_encrypt(void *ba
interim_len = vecs_data_len;
serf__log(LOGLVL_DEBUG, LOGCOMP_SSL, __FILE__, ctx->config,
- "ssl_encrypt: bucket read %"APR_SIZE_T_FMT" bytes; "\
+ "ssl_encrypt: bucket read %d bytes; "\
"status %d\n", interim_len, status);
/* When an SSL_write() operation has to be repeated because of
@@ -1386,8 +1391,8 @@ static apr_status_t ssl_encrypt(void *ba
serf_bucket_mem_free(ctx->allocator, vecs_data);
serf__log(LOGLVL_DEBUG, LOGCOMP_SSL, __FILE__, ctx->config,
- "---\n%.*s\n-(%"APR_SIZE_T_FMT")-\n",
- (int)interim_len, vecs_data, interim_len);
+ "---\n%.*s\n-(%d)-\n",
+ interim_len, vecs_data, interim_len);
}
}
@@ -1641,7 +1646,7 @@ static int ssl_need_client_cert(SSL *ssl
const char *cert_uri = NULL;
OSSL_STORE_CTX *store = NULL;
OSSL_STORE_INFO *info;
- X509 *c;
+ X509 *x509;
STACK_OF(X509_NAME) *requested;
int type;
@@ -1747,7 +1752,7 @@ static int ssl_need_client_cert(SSL *ssl
/* walk the leaf certificates, choose the best one */
- while ((c = sk_X509_pop(leaves))) {
+ while ((x509 = sk_X509_pop(leaves))) {
EVP_PKEY *k = NULL;
int i, n, found = 0;
@@ -1756,7 +1761,7 @@ static int ssl_need_client_cert(SSL *ssl
n = sk_EVP_PKEY_num(keys);
for (i = 0; i < n; ++i) {
k = sk_EVP_PKEY_value(keys, i);
- if (X509_check_private_key(c, k)) {
+ if (X509_check_private_key(x509, k)) {
found = 1;
break;
}
@@ -1767,10 +1772,10 @@ static int ssl_need_client_cert(SSL *ssl
/* CAs requested? if so, skip non matches, if not, accept all */
if (sk_X509_NAME_num(requested) &&
- !X509_get_ex_data(c, ssl_x509_ex_data_idx)) {
+ !X509_get_ex_data(x509, ssl_x509_ex_data_idx)) {
STACK_OF(X509) *chain;
- chain = X509_build_chain(c, intermediates, requests, 0, NULL,
+ chain = X509_build_chain(x509, intermediates, requests, 0,
NULL,
NULL);
if (!chain) {
@@ -1783,23 +1788,23 @@ static int ssl_need_client_cert(SSL *ssl
/* no best candidate yet? we're in first place */
if (!*cert) {
EVP_PKEY_up_ref(k);
- *cert = c; /* don't dup, we're returning this */
+ *cert = x509; /* don't dup, we're returning this */
*pkey = k;
continue;
}
/* were we issued after the previous best? */
if (ASN1_TIME_compare(X509_get0_notBefore(*cert),
- X509_get0_notBefore(c)) < 0) {
+ X509_get0_notBefore(x509)) < 0) {
X509_free(*cert);
EVP_PKEY_free(*pkey);
EVP_PKEY_up_ref(k);
- *cert = c; /* don't dup, we're returning this */
+ *cert = x509; /* don't dup, we're returning this */
*pkey = k;
continue;
}
- X509_free(c);
+ X509_free(x509);
}
break;
@@ -2243,8 +2248,9 @@ apr_status_t serf_ssl_set_hostname(serf_
ERR_clear_error();
}
return APR_SUCCESS;
-#endif
+#else
return APR_ENOTIMPL;
+#endif
}
apr_status_t serf_ssl_negotiate_protocol(serf_ssl_context_t *context,
@@ -2297,7 +2303,8 @@ apr_status_t serf_ssl_negotiate_protocol
at += len;
#ifdef SERF_HAVE_OPENSSL_ALPN
- if (SSL_set_alpn_protos(context->ssl, raw_header, raw_len)) {
+ /* Safe cast: raw_len < 65536 therefore raw_len <= INT_MAX */
+ if (SSL_set_alpn_protos(context->ssl, raw_header, (int)raw_len)) {
ERR_clear_error();
}
apr_pool_destroy(subpool);
@@ -2482,8 +2489,9 @@ serf_ssl_check_cert_status_request(serf_
SSL_CTX_set_tlsext_status_arg(ssl_ctx->ctx, ssl_ctx);
SSL_set_tlsext_status_type(ssl_ctx->ssl, TLSEXT_STATUSTYPE_ocsp);
return APR_SUCCESS;
-#endif
+#else
return APR_ENOTIMPL;
+#endif
}
serf_bucket_t *serf_bucket_ssl_decrypt_create(
@@ -2848,19 +2856,17 @@ static void disable_compression(serf_ssl
apr_status_t serf_ssl_use_compression(serf_ssl_context_t *ssl_ctx, int enabled)
{
- if (enabled) {
#ifdef SSL_OP_NO_COMPRESSION
+ if (enabled) {
SSL_clear_options(ssl_ctx->ssl, SSL_OP_NO_COMPRESSION);
return APR_SUCCESS;
-#endif
} else {
-#ifdef SSL_OP_NO_COMPRESSION
SSL_set_options(ssl_ctx->ssl, SSL_OP_NO_COMPRESSION);
return APR_SUCCESS;
-#endif
}
-
- return APR_EGENERAL;
+#else
+ return APR_ENOTIMPL;
+#endif
}
static void serf_ssl_destroy_and_data(serf_bucket_t *bucket)
@@ -3033,7 +3039,7 @@ struct serf_ssl_ocsp_request_t {
/* DER-encoded request and size. */
const void *der_request;
- apr_size_t der_request_size;
+ int der_request_size;
};
static apr_status_t free_ocsp_request(void *data)
@@ -3216,8 +3222,8 @@ serf_ssl_ocsp_request_t *serf_ssl_ocsp_r
const char *base64_request = apr_pstrmemdup(
scratch_pool, encoded_ocsp_request,
end_request - encoded_ocsp_request);
- long der_request_size = apr_base64_decode_len(base64_request);
- long der_id_size = apr_base64_decode_len(base64_id);
+ int der_request_size = apr_base64_decode_len(base64_request);
+ int der_id_size = apr_base64_decode_len(base64_id);
OCSP_REQUEST *ocsp_req;
OCSP_CERTID *cert_id;
Modified: serf/trunk/test/serf_get.c
URL:
http://svn.apache.org/viewvc/serf/trunk/test/serf_get.c?rev=1926988&r1=1926987&r2=1926988&view=diff
==============================================================================
--- serf/trunk/test/serf_get.c (original)
+++ serf/trunk/test/serf_get.c Sun Jul 6 09:52:17 2025
@@ -231,8 +231,8 @@ static apr_status_t conn_setup(apr_socke
if (!conn_ctx->ssl_ctx) {
conn_ctx->ssl_ctx = serf_bucket_ssl_decrypt_context_get(c);
}
- serf_ssl_server_cert_chain_callback_set(conn_ctx->ssl_ctx,
- ignore_all_cert_errors,
+ serf_ssl_server_cert_chain_callback_set(conn_ctx->ssl_ctx,
+ ignore_all_cert_errors,
print_certs, NULL);
serf_ssl_set_hostname(conn_ctx->ssl_ctx, ctx->hostname);
@@ -407,7 +407,7 @@ static apr_status_t setup_request(serf_r
*acceptor_baton = ctx->acceptor_baton;
*handler = ctx->handler;
*handler_baton = ctx;
-
+
return APR_SUCCESS;
}
@@ -816,7 +816,6 @@ int main(int argc, const char **argv)
if (debug)
{
serf_log_output_t *output;
- apr_status_t status;
status = serf_logging_create_stream_output(&output,
context,
Modified: serf/trunk/test/serf_httpd.c
URL:
http://svn.apache.org/viewvc/serf/trunk/test/serf_httpd.c?rev=1926988&r1=1926987&r2=1926988&view=diff
==============================================================================
--- serf/trunk/test/serf_httpd.c (original)
+++ serf/trunk/test/serf_httpd.c Sun Jul 6 09:52:17 2025
@@ -461,7 +461,6 @@ int main(int argc, const char **argv)
/* Setup debug logging */
if (verbose) {
serf_log_output_t *output;
- apr_status_t status;
apr_uint32_t level;
level = SERF_LOG_WARNING;
