From: James Jones Date: Mon, 31 Oct 2022 18:19:14 +0000 (-0500) Subject: Add -Wdeclaration-after-statement where supported (#4762) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3d70848781cb6ab4a638abf0580c165ba5c2f357;p=thirdparty%2Ffreeradius-server.git Add -Wdeclaration-after-statement where supported (#4762) * Add -Wdeclaration-after-statement where supported This enforces one of the FreeRADIUS coding standards. NOTE: I had to do a "make force-reconfig" to get the changes made to configure.ac and m4/ax_cc.m4 to have an effect in configure. This caused changes in thirty-five other configure files that are unrelated to this change. I am including only the changes to the files relevant to adding the above option to CFLAGS, namely the files I explicitly changed and configure itself. We also add changes to fix the declaration after statement instances this addition turns up. * Don't declare vars in loops * Don't declare vars in groups Co-authored-by: Arran Cudbard-Bell --- diff --git a/configure b/configure index 87177398d22..4eddbd39b8e 100755 --- a/configure +++ b/configure @@ -13119,6 +13119,61 @@ if test "x$developer" = "xyes"; then printf "%s\n" "$as_me: Setting additional developer CFLAGS" >&6;} + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for the compiler flag \"-Wdeclaration-after-statement\"" >&5 +printf %s "checking for the compiler flag \"-Wdeclaration-after-statement\"... " >&6; } +if test ${ax_cv_cc_wdeclaration_after_statement_flag+y} +then : + printf %s "(cached) " >&6 +else $as_nop + + + CFLAGS_SAVED=$CFLAGS + CFLAGS="$CFLAGS -Werror -Wdeclaration-after-statement" + + ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main (void) +{ +return 0; + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO" +then : + ax_cv_cc_wdeclaration_after_statement_flag="yes" +else $as_nop + ax_cv_cc_wdeclaration_after_statement_flag="no" +fi +rm -f core conftest.err conftest.$ac_objext conftest.beam conftest.$ac_ext + ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + + + CFLAGS="$CFLAGS_SAVED" + +fi +{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: $ax_cv_cc_wdeclaration_after_statement_flag" >&5 +printf "%s\n" "$ax_cv_cc_wdeclaration_after_statement_flag" >&6; } + + if test "x$ax_cv_cc_wdeclaration_after_statement_flag" = "xyes"; then + devcflags="$devcflags \ + -Wdeclaration-after-statement" + fi + + + { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for the compiler flag \"-Weverything\"" >&5 printf %s "checking for the compiler flag \"-Weverything\"... " >&6; } if test ${ax_cv_cc_weverything_flag+y} diff --git a/configure.ac b/configure.ac index 440f77167be..3772bffb1bf 100644 --- a/configure.ac +++ b/configure.ac @@ -1884,6 +1884,18 @@ AC_SUBST(LIBPREFIX) if test "x$developer" = "xyes"; then AC_MSG_NOTICE([Setting additional developer CFLAGS]) + dnl # + dnl # -Wdeclaration-after-statement is used where possible to insist + dnl # on the FreeRADIUS convention of putting declarations at the start + dnl # of the block they occur in. + dnl # + AX_CC_WDECLARATION_AFTER_STATEMENT_FLAG + if test "x$ax_cv_cc_wdeclaration_after_statement_flag" = "xyes"; then + devcflags="$devcflags \ + -Wdeclaration-after-statement" + fi + + dnl # dnl # If we have -Weverything, it really means *everything* unlike -Wall dnl # It's so verbose we need to turn off warnings which aren't useful. diff --git a/m4/ax_cc.m4 b/m4/ax_cc.m4 index 02743c2141d..88b69875e57 100644 --- a/m4/ax_cc.m4 +++ b/m4/ax_cc.m4 @@ -152,7 +152,23 @@ AC_DEFUN([AX_CC_NO_UNKNOWN_WARNING_OPTION_FLAG],[ ]) ]) +AC_DEFUN([AX_CC_WDECLARATION_AFTER_STATEMENT_FLAG],[ + AC_CACHE_CHECK([for the compiler flag "-Wdeclaration-after-statement"], [ax_cv_cc_wdeclaration_after_statement_flag],[ + CFLAGS_SAVED=$CFLAGS + CFLAGS="$CFLAGS -Werror -Wdeclaration-after-statement" + + AC_LANG_PUSH(C) + AC_TRY_COMPILE( + [], + [return 0;], + [ax_cv_cc_wdeclaration_after_statement_flag="yes"], + [ax_cv_cc_wdeclaration_after_statement_flag="no"]) + AC_LANG_POP + + CFLAGS="$CFLAGS_SAVED" + ]) +]) AC_DEFUN([AX_CC_WEVERYTHING_FLAG],[ AC_CACHE_CHECK([for the compiler flag "-Weverything"], [ax_cv_cc_weverything_flag],[ diff --git a/src/bin/collectd.c b/src/bin/collectd.c index cfbf946135c..3295f50e7d6 100644 --- a/src/bin/collectd.c +++ b/src/bin/collectd.c @@ -240,11 +240,9 @@ error: rs_stats_tmpl_t *rs_stats_collectd_init_latency(TALLOC_CTX *ctx, rs_stats_tmpl_t **out, rs_t *conf, char const *type, rs_latency_t *stats, fr_radius_packet_code_t code) { - rs_stats_tmpl_t **tmpl, *last; + rs_stats_tmpl_t **tmpl = out, *last; char *p; char buffer[LCC_NAME_LEN]; - tmpl = out; - rs_stats_value_tmpl_t rtx[(RS_RETRANSMIT_MAX + 1) + 1 + 1]; // RTX bins + 0 bin + lost + NULL int i; @@ -375,10 +373,10 @@ int rs_stats_collectd_open(rs_t *conf) */ int rs_stats_collectd_close(rs_t *conf) { - assert(conf->stats.collectd); - int ret = 0; + assert(conf->stats.collectd); + if (conf->stats.handle) { ret = lcc_disconnect(conf->stats.handle); conf->stats.handle = NULL; diff --git a/src/bin/dhcpclient.c b/src/bin/dhcpclient.c index 3c461736a75..77804bd31c4 100644 --- a/src/bin/dhcpclient.c +++ b/src/bin/dhcpclient.c @@ -322,8 +322,9 @@ static fr_radius_packet_t *fr_dhcpv4_recv_raw_loop(int lsockfd, /* display offer(s) received */ if (nb_offer > 0 ) { - DEBUG("Received %d DHCP Offer(s):", nb_offer); int i; + + DEBUG("Received %d DHCP Offer(s):", nb_offer); for (i = 0; i < nb_reply; i++) { char server_addr_buf[INET6_ADDRSTRLEN]; char offered_addr_buf[INET6_ADDRSTRLEN]; diff --git a/src/bin/radsniff.c b/src/bin/radsniff.c index 61cf6d1cd44..d0c1b22c339 100644 --- a/src/bin/radsniff.c +++ b/src/bin/radsniff.c @@ -2360,13 +2360,14 @@ int main(int argc, char *argv[]) { pcap_if_t *all_devices = NULL; pcap_if_t *dev_p; + int i; if (pcap_findalldevs(&all_devices, errbuf) < 0) { ERROR("Error getting available capture devices: %s", errbuf); goto finish; } - int i = 1; + i = 1; for (dev_p = all_devices; dev_p; dev_p = dev_p->next) { diff --git a/src/lib/ldap/base.c b/src/lib/ldap/base.c index cab4cb73c37..34df5e50915 100644 --- a/src/lib/ldap/base.c +++ b/src/lib/ldap/base.c @@ -534,6 +534,8 @@ fr_ldap_rcode_t fr_ldap_search_async(int *msgid, request_t *request, LDAPControl *our_serverctrls[LDAP_MAX_CONTROLS]; LDAPControl *our_clientctrls[LDAP_MAX_CONTROLS]; + char **search_attrs; + fr_ldap_control_merge(our_serverctrls, our_clientctrls, NUM_ELEMENTS(our_serverctrls), NUM_ELEMENTS(our_clientctrls), @@ -549,7 +551,6 @@ fr_ldap_rcode_t fr_ldap_search_async(int *msgid, request_t *request, * OpenLDAP library doesn't declare attrs array as const, but * it really should be *sigh*. */ - char **search_attrs; memcpy(&search_attrs, &attrs, sizeof(attrs)); if (filter) { @@ -631,8 +632,8 @@ static void ldap_trunk_query_cancel(UNUSED request_t *request, fr_state_signal_t #define SET_LDAP_CTRLS(_dest, _src) \ do { \ + int i; \ if (!_src) break; \ - int i; \ for (i = 0; i < LDAP_MAX_CONTROLS; i++) { \ if (!(_src[i])) break; \ _dest[i].control = _src[i]; \ diff --git a/src/lib/ldap/connection.c b/src/lib/ldap/connection.c index f7ed270cf68..35a74dfbce3 100644 --- a/src/lib/ldap/connection.c +++ b/src/lib/ldap/connection.c @@ -226,16 +226,18 @@ static int _ldap_connection_free(fr_ldap_connection_t *c) if (!c->handle) return 0; /* Don't need to do anything else if we don't yet have a handle */ - LDAPControl *our_serverctrls[LDAP_MAX_CONTROLS]; - LDAPControl *our_clientctrls[LDAP_MAX_CONTROLS]; + { + LDAPControl *our_serverctrls[LDAP_MAX_CONTROLS]; + LDAPControl *our_clientctrls[LDAP_MAX_CONTROLS]; - fr_ldap_control_merge(our_serverctrls, our_clientctrls, - NUM_ELEMENTS(our_serverctrls), - NUM_ELEMENTS(our_clientctrls), - c, NULL, NULL); + fr_ldap_control_merge(our_serverctrls, our_clientctrls, + NUM_ELEMENTS(our_serverctrls), + NUM_ELEMENTS(our_clientctrls), + c, NULL, NULL); - DEBUG3("Closing connection %p libldap handle %p", c->handle, c); - ldap_unbind_ext(c->handle, our_serverctrls, our_clientctrls); /* Same code as ldap_unbind_ext_s */ + DEBUG3("Closing connection %p libldap handle %p", c->handle, c); + ldap_unbind_ext(c->handle, our_serverctrls, our_clientctrls); /* Same code as ldap_unbind_ext_s */ + } c->handle = NULL; diff --git a/src/lib/server/command.c b/src/lib/server/command.c index d871c947a67..e7a8535eda5 100644 --- a/src/lib/server/command.c +++ b/src/lib/server/command.c @@ -297,9 +297,10 @@ static int split_alternation(char **input, char **output) if ((*str == '[') || (*str == '(')) { char end; - quote = *(str++); int count = 0; + quote = *(str++); + if (quote == '[') { end = ']'; } else { @@ -469,9 +470,10 @@ static int split(char **input, char **output, bool syntax_string) } else if (syntax_string && ((*str == '[') || (*str == '('))) { char end; - quote = *(str++); int count = 0; + quote = *(str++); + if (quote == '[') { end = ']'; } else { diff --git a/src/lib/server/connection.c b/src/lib/server/connection.c index 6f6752d16ac..19b418ba100 100644 --- a/src/lib/server/connection.c +++ b/src/lib/server/connection.c @@ -393,9 +393,11 @@ static inline void connection_watch_call(fr_connection_t *conn, fr_dlist_head_t #define WATCH_PRE(_conn) \ do { \ if (fr_dlist_empty(&(_conn)->watch_pre[(_conn)->pub.state])) break; \ - HANDLER_BEGIN(conn, &(_conn)->watch_pre[(_conn)->pub.state]); \ - connection_watch_call((_conn), &(_conn)->watch_pre[(_conn)->pub.state]); \ - HANDLER_END(conn); \ + { \ + HANDLER_BEGIN(conn, &(_conn)->watch_pre[(_conn)->pub.state]); \ + connection_watch_call((_conn), &(_conn)->watch_pre[(_conn)->pub.state]); \ + HANDLER_END(conn); \ + } \ } while(0) /** Call the post handler watch functions @@ -404,9 +406,11 @@ do { \ #define WATCH_POST(_conn) \ do { \ if (fr_dlist_empty(&(_conn)->watch_post[(_conn)->pub.state])) break; \ - HANDLER_BEGIN(conn, &(_conn)->watch_post[(_conn)->pub.state]); \ - connection_watch_call((_conn), &(_conn)->watch_post[(_conn)->pub.state]); \ - HANDLER_END(conn); \ + { \ + HANDLER_BEGIN(conn, &(_conn)->watch_post[(_conn)->pub.state]); \ + connection_watch_call((_conn), &(_conn)->watch_post[(_conn)->pub.state]); \ + HANDLER_END(conn); \ + } \ } while(0) /** Remove a watch function from a pre/post[state] list diff --git a/src/lib/server/map_async.c b/src/lib/server/map_async.c index b10b8fe0b6a..36ef7dffbbc 100644 --- a/src/lib/server/map_async.c +++ b/src/lib/server/map_async.c @@ -475,10 +475,11 @@ int map_to_list_mod(TALLOC_CTX *ctx, vp_list_mod_t **out, switch (mutated->rhs->type) { case TMPL_TYPE_XLAT: { - fr_assert(tmpl_xlat(mutated->rhs) != NULL); fr_dcursor_t from; fr_value_box_t *vb, *n_vb; + fr_assert(tmpl_xlat(mutated->rhs) != NULL); + assign_values: fr_assert(tmpl_is_attr(mutated->lhs)); fr_assert(tmpl_da(mutated->lhs)); /* We need to know which attribute to create */ diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c index 43d72e45c6c..bb6b7930479 100644 --- a/src/lib/server/tmpl_eval.c +++ b/src/lib/server/tmpl_eval.c @@ -899,11 +899,10 @@ int tmpl_copy_pairs(TALLOC_CTX *ctx, fr_pair_list_t *out, request_t *request, tm fr_pair_t *vp; fr_dcursor_t from; tmpl_dcursor_ctx_t cc; + int err; TMPL_VERIFY(vpt); - int err; - fr_assert(tmpl_is_attr(vpt) || tmpl_is_list(vpt)); for (vp = tmpl_dcursor_init(&err, NULL, &cc, &from, request, vpt); @@ -944,11 +943,10 @@ int tmpl_copy_pair_children(TALLOC_CTX *ctx, fr_pair_list_t *out, request_t *req fr_pair_t *vp; fr_dcursor_t from; tmpl_dcursor_ctx_t cc; + int err; TMPL_VERIFY(vpt); - int err; - fr_assert(tmpl_is_attr(vpt) || tmpl_is_list(vpt)); fr_pair_list_free(out); diff --git a/src/lib/tls/session.c b/src/lib/tls/session.c index ed8d169b368..9adea9a9cce 100644 --- a/src/lib/tls/session.c +++ b/src/lib/tls/session.c @@ -1772,10 +1772,9 @@ fr_tls_session_t *fr_tls_session_alloc_server(TALLOC_CTX *ctx, SSL_CTX *ssl_ctx, EVP_MD_CTX *md_ctx; uint8_t digest[SHA256_DIGEST_LENGTH]; - fr_assert(conf->cache.id_name); - static_assert(sizeof(digest) <= SSL_MAX_SSL_SESSION_ID_LENGTH, "SSL_MAX_SSL_SESSION_ID_LENGTH must be >= SHA256_DIGEST_LENGTH"); + fr_assert(conf->cache.id_name); if (tmpl_aexpand(tls_session, &context_id, request, conf->cache.id_name, NULL, NULL) < 0) { RPEDEBUG("Failed expanding session ID"); diff --git a/src/lib/tls/session.h b/src/lib/tls/session.h index 366d06f500e..3b654a987d0 100644 --- a/src/lib/tls/session.h +++ b/src/lib/tls/session.h @@ -196,11 +196,13 @@ static inline CC_HINT(nonnull) void _fr_tls_session_request_bind(char const *fil RDEBUG3("%s[%u] - Binding SSL * (%p) to request (%p)", file, line, ssl, request); #ifndef NDEBUG - request_t *old; - old = SSL_get_ex_data(ssl, FR_TLS_EX_INDEX_REQUEST); - if (old) { - (void)talloc_get_type_abort(ssl, request_t); - fr_assert(0); + { + request_t *old; + old = SSL_get_ex_data(ssl, FR_TLS_EX_INDEX_REQUEST); + if (old) { + (void)talloc_get_type_abort(ssl, request_t); + fr_assert(0); + } } #endif ret = SSL_set_ex_data(ssl, FR_TLS_EX_INDEX_REQUEST, request); diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index d26ecff976b..4f060ac159d 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -996,11 +996,10 @@ xlat_action_t xlat_frame_eval(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_exp_head_ xlat_action_t xa = XLAT_ACTION_DONE; xlat_exp_t const *node; fr_value_box_list_t result; /* tmp list so debug works correctly */ + fr_value_box_t *value; fr_value_box_list_init(&result); - fr_value_box_t *value; - *child = NULL; if (!*in) return XLAT_ACTION_DONE; diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index 71a8108fce8..1c9ed9835e8 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/src/lib/unlang/xlat_tokenize.c @@ -620,6 +620,7 @@ static inline int xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in, xlat_exp_t *node; fr_sbuff_marker_t m_s; + tmpl_rules_t our_t_rules; XLAT_DEBUG("ATTRIBUTE <-- %pV", fr_box_strvalue_len(fr_sbuff_current(in), fr_sbuff_remaining(in))); @@ -630,8 +631,6 @@ static inline int xlat_tokenize_attribute(xlat_exp_head_t *head, fr_sbuff_t *in, * and instead are "virtual" attributes like * Foreach-Variable-N. */ - tmpl_rules_t our_t_rules; - if (t_rules) { memset(&our_t_rules, 0, sizeof(our_t_rules)); our_t_rules = *t_rules; diff --git a/src/lib/util/acutest.h b/src/lib/util/acutest.h index 9caeab43562..eef4b365a29 100644 --- a/src/lib/util/acutest.h +++ b/src/lib/util/acutest.h @@ -1829,6 +1829,7 @@ int main(int argc, char** argv) { int i; + int index; acutest_argv0_ = argv[0]; @@ -1911,7 +1912,7 @@ main(int argc, char** argv) printf("1..%d\n", (int) acutest_count_); } - int index = acutest_worker_index_; + index = acutest_worker_index_; for(i = 0; acutest_list_[i].func != NULL; i++) { int run = (acutest_test_data_[i].flags & ACUTEST_FLAG_RUN_); if (acutest_skip_mode_) /* Run all tests except those listed. */ diff --git a/src/lib/util/debug.c b/src/lib/util/debug.c index 096ad089506..270dc87f2e0 100644 --- a/src/lib/util/debug.c +++ b/src/lib/util/debug.c @@ -803,45 +803,48 @@ int fr_set_dumpable(bool allow_core_dumps) dump_core = allow_core_dumps; #ifdef HAVE_SYS_RESOURCE_H - struct rlimit current; + { + struct rlimit current; - /* - * Reset the core limits (or disable them) - */ - if (getrlimit(RLIMIT_CORE, ¤t) < 0) { - fr_strerror_printf("Failed to get current core limit: %s", fr_syserror(errno)); - return -1; - } + /* + * Reset the core limits (or disable them) + */ + if (getrlimit(RLIMIT_CORE, ¤t) < 0) { + fr_strerror_printf("Failed to get current core limit: %s", fr_syserror(errno)); + return -1; + } - if (allow_core_dumps) { - if ((current.rlim_cur != init_core_limit.rlim_cur) || (current.rlim_max != init_core_limit.rlim_max)) { - if (setrlimit(RLIMIT_CORE, &init_core_limit) < 0) { - fr_strerror_printf("Cannot update core dump limit: %s", fr_syserror(errno)); + if (allow_core_dumps) { + if ((current.rlim_cur != init_core_limit.rlim_cur) || + (current.rlim_max != init_core_limit.rlim_max)) { + if (setrlimit(RLIMIT_CORE, &init_core_limit) < 0) { + fr_strerror_printf("Cannot update core dump limit: %s", fr_syserror(errno)); - return -1; + return -1; + } } - } - /* - * We've been told to disable core dumping, - * rlim_cur is not set to zero. - * - * Set rlim_cur to zero, but leave rlim_max - * set to whatever the current value is. - * - * This is because, later, we may need to - * re-enable core dumps to allow the debugger - * to attach *sigh*. - */ - } else if (current.rlim_cur != 0) { - struct rlimit no_core; + /* + * We've been told to disable core dumping, + * rlim_cur is not set to zero. + * + * Set rlim_cur to zero, but leave rlim_max + * set to whatever the current value is. + * + * This is because, later, we may need to + * re-enable core dumps to allow the debugger + * to attach *sigh*. + */ + } else if (current.rlim_cur != 0) { + struct rlimit no_core; - no_core.rlim_cur = 0; - no_core.rlim_max = current.rlim_max; + no_core.rlim_cur = 0; + no_core.rlim_max = current.rlim_max; - if (setrlimit(RLIMIT_CORE, &no_core) < 0) { - fr_strerror_printf("Failed disabling core dumps: %s", fr_syserror(errno)); + if (setrlimit(RLIMIT_CORE, &no_core) < 0) { + fr_strerror_printf("Failed disabling core dumps: %s", fr_syserror(errno)); - return -1; + return -1; + } } } #endif diff --git a/src/lib/util/inet.c b/src/lib/util/inet.c index 51494062ec6..7277dfa3697 100644 --- a/src/lib/util/inet.c +++ b/src/lib/util/inet.c @@ -1570,20 +1570,23 @@ int fr_interface_to_ethernet(char const *interface, fr_ethernet_t *ethernet) if (strcmp(i->ifa_name, interface) != 0) continue; #if defined(__linux__) || defined(__EMSCRIPTEN__) - struct sockaddr_ll *ll; + { + struct sockaddr_ll *ll; - ll = (struct sockaddr_ll *) i->ifa_addr; - if ((ll->sll_hatype != 1) || (ll->sll_halen != 6)) continue; - - memcpy(ethernet->addr, ll->sll_addr, 6); + ll = (struct sockaddr_ll *) i->ifa_addr; + if ((ll->sll_hatype != 1) || (ll->sll_halen != 6)) continue; + memcpy(ethernet->addr, ll->sll_addr, 6); + } #else - struct sockaddr_dl *ll; + { + struct sockaddr_dl *ll; - ll = (struct sockaddr_dl *) i->ifa_addr; - if (ll->sdl_alen != 6) continue; + ll = (struct sockaddr_dl *) i->ifa_addr; + if (ll->sdl_alen != 6) continue; - memcpy(ethernet->addr, LLADDR(ll), 6); + memcpy(ethernet->addr, LLADDR(ll), 6); + } #endif ret = 0; break; diff --git a/src/lib/util/lst_tests.c b/src/lib/util/lst_tests.c index 30fad15e1d4..d4cae4273c6 100644 --- a/src/lib/util/lst_tests.c +++ b/src/lib/util/lst_tests.c @@ -510,8 +510,10 @@ static void queue_cmp(unsigned int count) */ { lst_thing **array; + fr_time_t start_alloc, end_alloc, start_insert, end_insert, start_pop, end_pop, end_pop_first; + populate_values(values, count); - fr_time_t start_alloc, end_alloc, start_insert, end_insert, start_pop, end_pop, end_pop_first = fr_time_wrap(0); + end_pop_first = fr_time_wrap(0); start_alloc = fr_time(); array = talloc_array(NULL, lst_thing *, count); diff --git a/src/lib/util/minmax_heap_tests.c b/src/lib/util/minmax_heap_tests.c index fa9e8295c8f..fd812da1a9a 100644 --- a/src/lib/util/minmax_heap_tests.c +++ b/src/lib/util/minmax_heap_tests.c @@ -458,8 +458,10 @@ static void queue_cmp(unsigned int count) */ { minmax_heap_thing **array; + fr_time_t start_alloc, end_alloc, start_insert, end_insert, start_pop, end_pop, end_pop_first; + populate_values(values, count); - fr_time_t start_alloc, end_alloc, start_insert, end_insert, start_pop, end_pop, end_pop_first = fr_time_min(); + end_pop_first = fr_time_min(); start_alloc = fr_time(); array = talloc_array(NULL, minmax_heap_thing *, count); diff --git a/src/lib/util/net.c b/src/lib/util/net.c index 11822f26fd8..c0a4e7fc5c4 100644 --- a/src/lib/util/net.c +++ b/src/lib/util/net.c @@ -69,11 +69,11 @@ size_t fr_net_af_table_len = NUM_ELEMENTS(fr_net_af_table); /* * UDP header validation. */ - udp = (udp_header_t const *)data; uint16_t udp_len; ssize_t diff; uint16_t expected; + udp = (udp_header_t const *)data; udp_len = ntohs(udp->len); diff = udp_len - remaining; /* Truncated data */ diff --git a/src/lib/util/socket.c b/src/lib/util/socket.c index 4b4d630cdd8..46e24473183 100644 --- a/src/lib/util/socket.c +++ b/src/lib/util/socket.c @@ -917,70 +917,72 @@ int fr_socket_bind(int sockfd, fr_ipaddr_t const *src_ipaddr, uint16_t *src_port */ #else - struct ifaddrs *list = NULL; - bool bound = false; + { + struct ifaddrs *list = NULL; + bool bound = false; - /* - * Troll through all interfaces to see if there's - */ - if (getifaddrs(&list) == 0) { - struct ifaddrs *i; - - for (i = list; i != NULL; i = i->ifa_next) { - if (i->ifa_addr && i->ifa_name && (strcmp(i->ifa_name, interface) == 0)) { - /* - * IPv4, and there's either no src_ip, OR src_ip is INADDR_ANY, - * it's a match. - * - * We also update my_ipaddr to point to this particular IP, - * so that we can later bind() to it. This gets us the same - * effect as SO_BINDTODEVICE. - */ - if ((i->ifa_addr->sa_family == AF_INET) && - (!src_ipaddr || fr_ipaddr_is_inaddr_any(src_ipaddr))) { - (void) fr_ipaddr_from_sockaddr(&my_ipaddr, NULL, - (struct sockaddr_storage *) i->ifa_addr, - sizeof(struct sockaddr_in)); - my_ipaddr.scope_id = scope_id; - bound = true; - break; + /* + * Troll through all interfaces to see if there's + */ + if (getifaddrs(&list) == 0) { + struct ifaddrs *i; + + for (i = list; i != NULL; i = i->ifa_next) { + if (i->ifa_addr && i->ifa_name && (strcmp(i->ifa_name, interface) == 0)) { + /* + * IPv4, and there's either no src_ip, OR src_ip is INADDR_ANY, + * it's a match. + * + * We also update my_ipaddr to point to this particular IP, + * so that we can later bind() to it. This gets us the same + * effect as SO_BINDTODEVICE. + */ + if ((i->ifa_addr->sa_family == AF_INET) && + (!src_ipaddr || fr_ipaddr_is_inaddr_any(src_ipaddr))) { + (void) fr_ipaddr_from_sockaddr(&my_ipaddr, NULL, + (struct sockaddr_storage *) i->ifa_addr, + sizeof(struct sockaddr_in)); + my_ipaddr.scope_id = scope_id; + bound = true; + break; + } + + /* + * The caller specified a source IP, and we find a matching + * address family. Allow it. + * + * Note that we do NOT check for matching IPs here. If we did, + * then binding to an interface and the *wrong* IP would get us + * a "bind to device is unsupported" message. + * + * Instead we say "yes, we found a matching interface", and then + * allow the bind() call below to run. If that fails, we get a + * "Can't assign requested address" error, which is more informative. + */ + if (src_ipaddr && (src_ipaddr->af == i->ifa_addr->sa_family)) { + my_ipaddr.scope_id = scope_id; + bound = true; + break; + } } + } + freeifaddrs(list); + + if (!bound) { /* - * The caller specified a source IP, and we find a matching - * address family. Allow it. - * - * Note that we do NOT check for matching IPs here. If we did, - * then binding to an interface and the *wrong* IP would get us - * a "bind to device is unsupported" message. - * - * Instead we say "yes, we found a matching interface", and then - * allow the bind() call below to run. If that fails, we get a - * "Can't assign requested address" error, which is more informative. + * IPv4: no link local addresses, + * and no bind to device. */ - if (src_ipaddr && (src_ipaddr->af == i->ifa_addr->sa_family)) { - my_ipaddr.scope_id = scope_id; - bound = true; - break; - } + fr_strerror_printf_push("Bind to interface %s failed: Unable to match " + "interface with the given IP address.", interface); + return -1; } - } - - freeifaddrs(list); - - if (!bound) { - /* - * IPv4: no link local addresses, - * and no bind to device. - */ - fr_strerror_printf_push("Bind to interface %s failed: Unable to match interface with the given IP address.", - interface); + } else { + fr_strerror_printf_push("Bind to interface %s failed, unable to get list of interfaces: %s", + interface, fr_syserror(errno)); return -1; } - } else { - fr_strerror_printf_push("Bind to interface %s failed, unable to get list of interfaces: %s", - interface, fr_syserror(errno)); - return -1; } #endif } /* else no interface was passed in */ diff --git a/src/lib/util/value.c b/src/lib/util/value.c index d16ac41736a..df2f1e18064 100644 --- a/src/lib/util/value.c +++ b/src/lib/util/value.c @@ -2454,10 +2454,9 @@ static inline int fr_value_box_cast_to_ipv6addr(TALLOC_CTX *ctx, fr_value_box_t fr_type_t dst_type, fr_dict_attr_t const *dst_enumv, fr_value_box_t const *src) { - fr_assert(dst_type == FR_TYPE_IPV6_ADDR); - static_assert((sizeof(v4_v6_map) + sizeof(src->vb_ip.addr.v4)) <= sizeof(src->vb_ip.addr.v6), "IPv6 storage too small"); + fr_assert(dst_type == FR_TYPE_IPV6_ADDR); switch (src->type) { case FR_TYPE_STRING: @@ -5430,20 +5429,22 @@ ssize_t fr_value_box_list_concat_as_octets(bool *tainted, fr_dbuff_t *dbuff, fr_ default: cast: - if (!tmp_ctx) tmp_ctx = talloc_pool(NULL, 1024); - fr_value_box_t tmp_vb; + { + fr_value_box_t tmp_vb; - /* - * Not equivalent to fr_value_box_to_network - */ - if (fr_value_box_cast_to_octets(tmp_ctx, &tmp_vb, FR_TYPE_OCTETS, NULL, vb) < 0) { - slen = -1; - goto error; - } + if (!tmp_ctx) tmp_ctx = talloc_pool(NULL, 1024); + /* + * Not equivalent to fr_value_box_to_network + */ + if (fr_value_box_cast_to_octets(tmp_ctx, &tmp_vb, FR_TYPE_OCTETS, NULL, vb) < 0) { + slen = -1; + goto error; + } - slen = fr_dbuff_in_memcpy(&our_dbuff, tmp_vb.vb_octets, tmp_vb.vb_length); - fr_value_box_clear_value(&tmp_vb); - break; + slen = fr_dbuff_in_memcpy(&our_dbuff, tmp_vb.vb_octets, tmp_vb.vb_length); + fr_value_box_clear_value(&tmp_vb); + break; + } } if (slen < 0) { diff --git a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c index 7e8b81ed4a9..3722ff2d400 100644 --- a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c +++ b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c @@ -921,9 +921,10 @@ fr_radius_packet_code_t eap_fast_process(request_t *request, eap_session_t *eap_ if (!eap_fast_verify(request, tls_session, data, data_len)) return FR_RADIUS_CODE_ACCESS_REJECT; if (t->stage == EAP_FAST_TLS_SESSION_HANDSHAKE) { + char buf[256]; + fr_assert(t->mode == EAP_FAST_UNKNOWN); - char buf[256]; if (strstr(SSL_CIPHER_description(SSL_get_current_cipher(tls_session->ssl), buf, sizeof(buf)), "Au=None")) { /* FIXME enforce MSCHAPv2 - RFC 5422 section 3.2.2 */ diff --git a/src/modules/rlm_eap/types/rlm_eap_peap/peap.c b/src/modules/rlm_eap/types/rlm_eap_peap/peap.c index ad175837794..10f1b26a4a5 100644 --- a/src/modules/rlm_eap/types/rlm_eap_peap/peap.c +++ b/src/modules/rlm_eap/types/rlm_eap_peap/peap.c @@ -303,9 +303,10 @@ static void eap_peap_inner_to_pairs(TALLOC_CTX *ctx, fr_pair_list_t *pairs, */ static int eap_peap_inner_from_pairs(request_t *request, fr_tls_session_t *tls_session, fr_pair_list_t *vps) { - fr_assert(!fr_pair_list_empty(vps)); fr_pair_t *this; + fr_assert(!fr_pair_list_empty(vps)); + /* * Send the EAP data in the first attribute, WITHOUT the * header. diff --git a/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c b/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c index b66ab172922..49218a48058 100644 --- a/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c +++ b/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c @@ -256,9 +256,10 @@ redo: static int skip_spaces(rlm_isc_dhcp_tokenizer_t *state, char *p) { - state->ptr = p; char *start = p; + state->ptr = p; + fr_skip_whitespace(state->ptr); /* diff --git a/src/modules/rlm_perl/rlm_perl.c b/src/modules/rlm_perl/rlm_perl.c index b66317d0f82..e220fa2bba0 100644 --- a/src/modules/rlm_perl/rlm_perl.c +++ b/src/modules/rlm_perl/rlm_perl.c @@ -546,16 +546,14 @@ static xlat_action_t perl_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, */ static void perl_parse_config(CONF_SECTION *cs, int lvl, HV *rad_hv) { - if (!cs || !rad_hv) return; - int indent_section = (lvl + 1) * 4; int indent_item = (lvl + 2) * 4; - DEBUG("%*s%s {", indent_section, " ", cf_section_name1(cs)); + if (!cs || !rad_hv) return; - CONF_ITEM *ci = NULL; + DEBUG("%*s%s {", indent_section, " ", cf_section_name1(cs)); - while ((ci = cf_item_next(cs, ci))) { + for (CONF_ITEM *ci = NULL; (ci = cf_item_next(cs, ci)); ) { /* * This is a section. * Create a new HV, store it as a reference in current HV, @@ -665,11 +663,10 @@ static void perl_store_vps(UNUSED TALLOC_CTX *ctx, request_t *request, fr_pair_l const char *hash_name, const char *list_name) { fr_pair_t *vp; + fr_dcursor_t cursor; hv_undef(rad_hv); - fr_dcursor_t cursor; - RINDENT(); fr_pair_list_sort(vps, fr_pair_cmp_by_da); for (vp = fr_pair_dcursor_init(&cursor, vps); diff --git a/src/modules/rlm_rest/rest.c b/src/modules/rlm_rest/rest.c index 3d452570325..1c7cff69f05 100644 --- a/src/modules/rlm_rest/rest.c +++ b/src/modules/rlm_rest/rest.c @@ -1004,170 +1004,173 @@ static int json_pair_alloc(rlm_rest_t const *instance, rlm_rest_section_t const /* * Process VP container */ - json_object_object_foreach(object, name, value) { - int i = 0, elements; - struct json_object *element, *tmp; - TALLOC_CTX *ctx; - - json_flags_t flags = { - .op = T_OP_SET, - .do_xlat = 1, - .is_json = 0 - }; - - request_t *current = request; - fr_pair_list_t *vps; - fr_pair_t *vp = NULL; - - TALLOC_FREE(dst); - - /* - * Resolve attribute name to a dictionary entry and pairlist. - */ - RDEBUG2("Parsing attribute \"%s\"", name); - - if (tmpl_afrom_attr_str(request, NULL, &dst, name, - &(tmpl_rules_t){ - .attr = { - .prefix = TMPL_ATTR_REF_PREFIX_NO, - .dict_def = request->dict, - .list_def = PAIR_LIST_REPLY - } - }) <= 0) { - RPWDEBUG("Failed parsing attribute (skipping)"); - continue; - } + { + json_object_object_foreach(object, name, value) { + int i = 0, elements; + struct json_object *element, *tmp; + TALLOC_CTX *ctx; - if (tmpl_request_ptr(¤t, tmpl_request(dst)) < 0) { - RWDEBUG("Attribute name refers to outer request but not in a tunnel (skipping)"); - continue; - } + json_flags_t flags = { + .op = T_OP_SET, + .do_xlat = 1, + .is_json = 0 + }; - vps = tmpl_list_head(current, tmpl_list(dst)); - if (!vps) { - RWDEBUG("List not valid in this context (skipping)"); - continue; - } - ctx = tmpl_list_ctx(current, tmpl_list(dst)); + request_t *current = request; + fr_pair_list_t *vps; + fr_pair_t *vp = NULL; - /* - * Alternative JSON structure which allows operator, - * and other flags to be specified. - * - * "":{ - * "do_xlat":, - * "is_json":, - * "op":"", - * "value": - * } - * - * Where value is a: - * - [] Multivalued array - * - {} Nested Valuepair - * - * Integer or string value - */ - if (json_object_is_type(value, json_type_object)) { - /* - * Process operator if present. - */ - if (json_object_object_get_ex(value, "op", &tmp)) { - flags.op = fr_table_value_by_str(fr_tokens_table, json_object_get_string(tmp), 0); - if (!flags.op) { - RWDEBUG("Invalid operator value \"%s\" (skipping)", - json_object_get_string(tmp)); - continue; - } - } + TALLOC_FREE(dst); /* - * Process optional do_xlat bool. + * Resolve attribute name to a dictionary entry and pairlist. */ - if (json_object_object_get_ex(value, "do_xlat", &tmp)) { - flags.do_xlat = json_object_get_boolean(tmp); + RDEBUG2("Parsing attribute \"%s\"", name); + + if (tmpl_afrom_attr_str(request, NULL, &dst, name, + &(tmpl_rules_t){ + .attr = { + .prefix = TMPL_ATTR_REF_PREFIX_NO, + .dict_def = request->dict, + .list_def = PAIR_LIST_REPLY + } + }) <= 0) { + RPWDEBUG("Failed parsing attribute (skipping)"); + continue; } - /* - * Process optional is_json bool. - */ - if (json_object_object_get_ex(value, "is_json", &tmp)) { - flags.is_json = json_object_get_boolean(tmp); + if (tmpl_request_ptr(¤t, tmpl_request(dst)) < 0) { + RWDEBUG("Attribute name refers to outer request but not in a tunnel (skipping)"); + continue; } - /* - * Value key must be present if were using the expanded syntax. - */ - if (!json_object_object_get_ex(value, "value", &tmp)) { - RWDEBUG("Value key missing (skipping)"); + vps = tmpl_list_head(current, tmpl_list(dst)); + if (!vps) { + RWDEBUG("List not valid in this context (skipping)"); continue; } + ctx = tmpl_list_ctx(current, tmpl_list(dst)); /* - * The value field now becomes the key we're operating on + * Alternative JSON structure which allows operator, + * and other flags to be specified. + * + * "":{ + * "do_xlat":, + * "is_json":, + * "op":"", + * "value": + * } + * + * Where value is a: + * - [] Multivalued array + * - {} Nested Valuepair + * - * Integer or string value */ - value = tmp; - } + if (json_object_is_type(value, json_type_object)) { + /* + * Process operator if present. + */ + if (json_object_object_get_ex(value, "op", &tmp)) { + flags.op = fr_table_value_by_str(fr_tokens_table, json_object_get_string(tmp), 0); + if (!flags.op) { + RWDEBUG("Invalid operator value \"%s\" (skipping)", + json_object_get_string(tmp)); + continue; + } + } - /* - * Setup fr_pair_afrom_da / recursion loop. - */ - if (!flags.is_json && json_object_is_type(value, json_type_array)) { - elements = json_object_array_length(value); - if (!elements) { - RWDEBUG("Zero length value array (skipping)"); - continue; - } - element = json_object_array_get_idx(value, 0); - } else { - elements = 1; - element = value; - } + /* + * Process optional do_xlat bool. + */ + if (json_object_object_get_ex(value, "do_xlat", &tmp)) { + flags.do_xlat = json_object_get_boolean(tmp); + } - /* - * A JSON 'value' key, may have multiple elements, iterate - * over each of them, creating a new fr_pair_t. - */ - do { - if (max_attrs-- <= 0) { - RWDEBUG("At maximum attribute limit"); - talloc_free(dst); - return max; + /* + * Process optional is_json bool. + */ + if (json_object_object_get_ex(value, "is_json", &tmp)) { + flags.is_json = json_object_get_boolean(tmp); + } + + /* + * Value key must be present if were using the expanded syntax. + */ + if (!json_object_object_get_ex(value, "value", &tmp)) { + RWDEBUG("Value key missing (skipping)"); + continue; + } + + /* + * The value field now becomes the key we're operating on + */ + value = tmp; } /* - * Automagically switch the op for multivalued attributes. + * Setup fr_pair_afrom_da / recursion loop. */ - if (((flags.op == T_OP_SET) || (flags.op == T_OP_EQ)) && (i >= 1)) { - flags.op = T_OP_ADD_EQ; + if (!flags.is_json && json_object_is_type(value, json_type_array)) { + elements = json_object_array_length(value); + if (!elements) { + RWDEBUG("Zero length value array (skipping)"); + continue; + } + element = json_object_array_get_idx(value, 0); + } else { + elements = 1; + element = value; } - if (json_object_is_type(element, json_type_object) && !flags.is_json) { - /* TODO: Insert nested VP into VP structure...*/ - RWDEBUG("Found nested VP, these are not yet supported (skipping)"); + /* + * A JSON 'value' key, may have multiple elements, iterate + * over each of them, creating a new fr_pair_t. + */ + do { + fr_pair_list_t tmp_list; - continue; + if (max_attrs-- <= 0) { + RWDEBUG("At maximum attribute limit"); + talloc_free(dst); + return max; + } /* - vp = json_pair_alloc(instance, section, - request, value, - level + 1, max_attrs);*/ - } else { - vp = json_pair_alloc_leaf(instance, section, ctx, request, - tmpl_da(dst), &flags, element); - if (!vp) continue; - } - RINDENT(); - RDEBUG2("&%s:%pP", fr_table_str_by_value(pair_list_table, tmpl_list(dst), ""), vp); - REXDENT(); + * Automagically switch the op for multivalued attributes. + */ + if (((flags.op == T_OP_SET) || (flags.op == T_OP_EQ)) && (i >= 1)) { + flags.op = T_OP_ADD_EQ; + } - fr_pair_list_t tmp_list; - fr_pair_list_init(&tmp_list); - fr_pair_append(&tmp_list, vp); - radius_pairmove(current, vps, &tmp_list); - /* - * If we call json_object_array_get_idx on something that's not an array - * the behaviour appears to be to occasionally segfault. - */ - } while ((++i < elements) && (element = json_object_array_get_idx(value, i))); + if (json_object_is_type(element, json_type_object) && !flags.is_json) { + /* TODO: Insert nested VP into VP structure...*/ + RWDEBUG("Found nested VP, these are not yet supported (skipping)"); + + continue; + + /* + vp = json_pair_alloc(instance, section, + request, value, + level + 1, max_attrs);*/ + } else { + vp = json_pair_alloc_leaf(instance, section, ctx, request, + tmpl_da(dst), &flags, element); + if (!vp) continue; + } + RINDENT(); + RDEBUG2("&%s:%pP", fr_table_str_by_value(pair_list_table, tmpl_list(dst), ""), vp); + REXDENT(); + + fr_pair_list_init(&tmp_list); + fr_pair_append(&tmp_list, vp); + radius_pairmove(current, vps, &tmp_list); + /* + * If we call json_object_array_get_idx on something that's not an array + * the behaviour appears to be to occasionally segfault. + */ + } while ((++i < elements) && (element = json_object_array_get_idx(value, i))); + } } talloc_free(dst); diff --git a/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_fbapi.c b/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_fbapi.c index 876ede787b1..50a19870718 100644 --- a/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_fbapi.c +++ b/src/modules/rlm_sql/drivers/rlm_sql_firebird/sql_fbapi.c @@ -516,14 +516,16 @@ int fb_affected_rows(rlm_sql_firebird_conn_t *conn) { p = info_buffer + 3; while (*p != isc_info_end) { p++; - short len = (short)isc_vax_integer(p, 2); - p += 2; + { + short len = (short)isc_vax_integer(p, 2); + p += 2; - affected_rows = isc_vax_integer(p, len); - if (affected_rows > 0) { - break; + affected_rows = isc_vax_integer(p, len); + if (affected_rows > 0) { + break; + } + p += len; } - p += len; } return affected_rows; } diff --git a/src/protocols/dhcpv4/raw.c b/src/protocols/dhcpv4/raw.c index a92fddba9c1..79539d273e4 100644 --- a/src/protocols/dhcpv4/raw.c +++ b/src/protocols/dhcpv4/raw.c @@ -184,6 +184,7 @@ fr_radius_packet_t *fr_dhcv4_raw_packet_recv(int sockfd, struct sockaddr_ll *lin uint16_t udp_dst_port; size_t dhcp_data_len; socklen_t sock_len; + uint8_t data_offset; packet = fr_radius_packet_alloc(NULL, false); if (!packet) { @@ -204,7 +205,7 @@ fr_radius_packet_t *fr_dhcv4_raw_packet_recv(int sockfd, struct sockaddr_ll *lin sock_len = sizeof(struct sockaddr_ll); data_len = recvfrom(sockfd, raw_packet, MAX_PACKET_SIZE, 0, (struct sockaddr *)link_layer, &sock_len); - uint8_t data_offset = ETH_HDR_SIZE + IP_HDR_SIZE + UDP_HDR_SIZE; /* DHCP data datas after Ethernet, IP, UDP */ + data_offset = ETH_HDR_SIZE + IP_HDR_SIZE + UDP_HDR_SIZE; /* DHCP data datas after Ethernet, IP, UDP */ if (data_len <= data_offset) DISCARD_RP("Payload (%d) smaller than required for layers 2+3+4", (int)data_len); diff --git a/src/protocols/tacacs/base.c b/src/protocols/tacacs/base.c index 59827d4674d..bb6328362ea 100644 --- a/src/protocols/tacacs/base.c +++ b/src/protocols/tacacs/base.c @@ -132,6 +132,7 @@ int fr_tacacs_body_xor(fr_tacacs_packet_t const *pkt, uint8_t *body, size_t body uint8_t pad[MD5_DIGEST_LENGTH]; uint8_t *buf; int pad_offset; + size_t pos; if (!secret) { if (pkt->hdr.flags & FR_TAC_PLUS_UNENCRYPTED_FLAG) @@ -160,7 +161,7 @@ int fr_tacacs_body_xor(fr_tacacs_packet_t const *pkt, uint8_t *body, size_t body fr_md5_calc(pad, buf, pad_offset); - size_t pos = 0; + pos = 0; do { for (size_t i = 0; i < MD5_DIGEST_LENGTH && pos < body_len; i++, pos++) body[pos] ^= pad[i];