From 14040553bc8db794de5f44b622c20ff8437479ff Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 31 Aug 2021 23:14:10 +0200 Subject: [PATCH 1/7] lib: fix hook defs for -Wstrict-prototypes Without this, the hook code creates functions with empty parameter lists like "void hook_something()", which is not a proper C prototype. It needs to be "void hook_something(void)". Add some macro shenanigans to handle that. ... and make the plumbing functions "inline" too. Signed-off-by: David Lamparter --- lib/hook.h | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/hook.h b/lib/hook.h index ff3ef29fa3..3a0db6009b 100644 --- a/lib/hook.h +++ b/lib/hook.h @@ -183,6 +183,12 @@ extern void _hook_unregister(struct hook *hook, void *funcptr, void *arg, #define HOOK_ADDDEF(...) (void *hookarg , ## __VA_ARGS__) #define HOOK_ADDARG(...) (hookarg , ## __VA_ARGS__) +/* and another helper to convert () into (void) to get a proper prototype */ +#define _SKIP_10(x0, x1, x2, x3, x4, x5, x6, x7, x8, x9, ret, ...) ret +#define _MAKE_VOID(...) _SKIP_10(, ##__VA_ARGS__, , , , , , , , , , void) + +#define HOOK_VOIDIFY(...) (_MAKE_VOID(__VA_ARGS__) __VA_ARGS__) + /* use in header file - declares the hook and its arguments * usage: DECLARE_HOOK(my_hook, (int arg1, struct foo *arg2), (arg1, arg2)); * as above, "passlist" must use the same order and same names as "arglist" @@ -192,13 +198,14 @@ extern void _hook_unregister(struct hook *hook, void *funcptr, void *arg, */ #define DECLARE_HOOK(hookname, arglist, passlist) \ extern struct hook _hook_##hookname; \ - __attribute__((unused)) static void *_hook_typecheck_##hookname( \ - int(*funcptr) arglist) \ + __attribute__((unused)) static inline void * \ + _hook_typecheck_##hookname(int(*funcptr) HOOK_VOIDIFY arglist) \ { \ return (void *)funcptr; \ } \ - __attribute__((unused)) static void *_hook_typecheck_arg_##hookname( \ - int(*funcptr) HOOK_ADDDEF arglist) \ + __attribute__((unused)) static inline void \ + *_hook_typecheck_arg_##hookname(int(*funcptr) \ + HOOK_ADDDEF arglist) \ { \ return (void *)funcptr; \ } \ @@ -213,14 +220,14 @@ extern void _hook_unregister(struct hook *hook, void *funcptr, void *arg, struct hook _hook_##hookname = { \ .name = #hookname, .entries = NULL, .reverse = rev, \ }; \ - static int hook_call_##hookname arglist \ + static int hook_call_##hookname HOOK_VOIDIFY arglist \ { \ int hooksum = 0; \ struct hookent *he = _hook_##hookname.entries; \ void *hookarg; \ union { \ void *voidptr; \ - int(*fptr) arglist; \ + int(*fptr) HOOK_VOIDIFY arglist; \ int(*farg) HOOK_ADDDEF arglist; \ } hookp; \ for (; he; he = he->next) { \ From c17662db538387b6b0244a59f66d86acfbac97cd Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 31 Aug 2021 23:17:15 +0200 Subject: [PATCH 2/7] pceplib: fix for -Wstrict-prototypes Just some "void" missing between empty braces. Signed-off-by: David Lamparter --- pceplib/pcep_msg_tlvs_encoding.c | 2 +- pceplib/pcep_session_logic.c | 4 ++-- pceplib/pcep_timers.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pceplib/pcep_msg_tlvs_encoding.c b/pceplib/pcep_msg_tlvs_encoding.c index d59c97c9da..c46e859c49 100644 --- a/pceplib/pcep_msg_tlvs_encoding.c +++ b/pceplib/pcep_msg_tlvs_encoding.c @@ -250,7 +250,7 @@ struct pcep_object_tlv_header *(*const tlv_decoders[MAX_TLV_ENCODER_INDEX])( [PCEP_OBJ_TLV_TYPE_OBJECTIVE_FUNCTION_LIST] = pcep_decode_tlv_of_list, }; -static void initialize_tlv_coders() +static void initialize_tlv_coders(void) { static bool initialized = false; diff --git a/pceplib/pcep_session_logic.c b/pceplib/pcep_session_logic.c index 2ec2fd72a8..ce898d1bf5 100644 --- a/pceplib/pcep_session_logic.c +++ b/pceplib/pcep_session_logic.c @@ -52,7 +52,7 @@ int session_id_ = 0; void send_pcep_open(pcep_session *session); /* forward decl */ -static bool run_session_logic_common() +static bool run_session_logic_common(void) { if (session_logic_handle_ != NULL) { pcep_log(LOG_WARNING, @@ -369,7 +369,7 @@ void pcep_session_cancel_timers(pcep_session *session) } /* Internal util function */ -static int get_next_session_id() +static int get_next_session_id(void) { if (session_id_ == INT_MAX) { session_id_ = 0; diff --git a/pceplib/pcep_timers.c b/pceplib/pcep_timers.c index 4c06d2b3f7..bbf9b77983 100644 --- a/pceplib/pcep_timers.c +++ b/pceplib/pcep_timers.c @@ -75,7 +75,7 @@ int timer_list_node_timer_ptr_compare(void *list_entry, void *new_entry) } /* internal util method */ -static pcep_timers_context *create_timers_context_() +static pcep_timers_context *create_timers_context_(void) { if (timers_context_ == NULL) { timers_context_ = pceplib_malloc(PCEPLIB_INFRA, From 9fb83ab17d5135dad229841f43651a1aa3cd6578 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 31 Aug 2021 23:16:57 +0200 Subject: [PATCH 3/7] *: fix for -Wstrict-prototypes Just some "void" missing between empty braces. Signed-off-by: David Lamparter --- ldpd/ldp_snmp.c | 2 +- lib/getopt.c | 5 ++--- ospfd/ospf_sr.c | 2 +- pathd/path_pcep_cli.c | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/ldpd/ldp_snmp.c b/ldpd/ldp_snmp.c index 3932df48e0..dfc7d145fe 100644 --- a/ldpd/ldp_snmp.c +++ b/ldpd/ldp_snmp.c @@ -1166,7 +1166,7 @@ ldpTrapSessionDown(struct nbr * nbr) ldpTrapSession(nbr, LDPSESSIONDOWN); } -static int ldp_snmp_agentx_enabled() +static int ldp_snmp_agentx_enabled(void) { main_imsg_compose_both(IMSG_AGENTX_ENABLED, NULL, 0); diff --git a/lib/getopt.c b/lib/getopt.c index 71799c9b6d..a33d196015 100644 --- a/lib/getopt.c +++ b/lib/getopt.c @@ -206,11 +206,10 @@ static char *posixly_correct; whose names are inconsistent. */ #ifndef getenv -extern char *getenv(); +extern char *getenv(const char *); #endif -static char *my_index(str, chr) const char *str; -int chr; +static char *my_index(const char *str, int chr) { while (*str) { if (*str == chr) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index bf2c5f3c40..9a9e64cc23 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -305,7 +305,7 @@ static int sr_local_block_init(uint32_t lower_bound, uint32_t upper_bound) * Remove Segment Routing Local Block. * */ -static void sr_local_block_delete() +static void sr_local_block_delete(void) { struct sr_local_block *srlb = &OspfSR.srlb; diff --git a/pathd/path_pcep_cli.c b/pathd/path_pcep_cli.c index 829df3179c..a6f253d3e3 100644 --- a/pathd/path_pcep_cli.c +++ b/pathd/path_pcep_cli.c @@ -69,7 +69,7 @@ static int pcep_cli_pcep_pce_config_write(struct vty *vty); /* Internal Util Function declarations */ static struct pce_opts_cli *pcep_cli_find_pce(const char *pce_name); static bool pcep_cli_add_pce(struct pce_opts_cli *pce_opts_cli); -static struct pce_opts_cli *pcep_cli_create_pce_opts(); +static struct pce_opts_cli *pcep_cli_create_pce_opts(const char *name); static void pcep_cli_delete_pce(const char *pce_name); static void pcep_cli_merge_pcep_pce_config_options(struct pce_opts_cli *pce_opts_cli); From 9c1490843a0cda6caed810aeaded77858d7d2089 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 2 Sep 2021 12:20:56 +0200 Subject: [PATCH 4/7] build: ignore prototype warnings from readline Readline contains some truly ancient cruft. Signed-off-by: David Lamparter --- vtysh/vtysh.c | 4 ++++ vtysh/vtysh_main.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index b47cca76f5..b74360c75f 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -26,8 +26,12 @@ #include #include +/* readline carries some ancient definitions around */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstrict-prototypes" #include #include +#pragma GCC diagnostic pop #include #include diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c index 20be81b901..76956574cc 100644 --- a/vtysh/vtysh_main.c +++ b/vtysh/vtysh_main.c @@ -27,8 +27,12 @@ #include #include +/* readline carries some ancient definitions around */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstrict-prototypes" #include #include +#pragma GCC diagnostic pop /* * The append_history function only appears in newer versions From 09e33fbe6b1a1d0f9b77c901dd916f95269d76b1 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 1 Sep 2021 17:12:15 +0200 Subject: [PATCH 5/7] build: enable `-Wstrict-prototypes` All fixed up, so we can enable this now. Signed-off-by: David Lamparter --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index c86f47d073..cf1c1e499c 100644 --- a/configure.ac +++ b/configure.ac @@ -322,6 +322,7 @@ AC_C_FLAG([-fno-omit-frame-pointer]) AC_C_FLAG([-funwind-tables]) AC_C_FLAG([-Wall]) AC_C_FLAG([-Wextra]) +AC_C_FLAG([-Wstrict-prototypes]) AC_C_FLAG([-Wmissing-prototypes]) AC_C_FLAG([-Wmissing-declarations]) AC_C_FLAG([-Wpointer-arith]) @@ -330,7 +331,6 @@ AC_C_FLAG([-Wwrite-strings]) AC_C_FLAG([-Wundef]) if test "$enable_gcc_ultra_verbose" = "yes" ; then AC_C_FLAG([-Wcast-qual]) - AC_C_FLAG([-Wstrict-prototypes]) AC_C_FLAG([-Wmissing-noreturn]) AC_C_FLAG([-Wmissing-format-attribute]) AC_C_FLAG([-Wunreachable-code]) From e5af0fc869281a96bca00e7d77680800af163663 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Wed, 1 Sep 2021 17:13:09 +0200 Subject: [PATCH 6/7] doc/developer: add warning pointers These two warnings are easy to get confused by, note down some pointers on what they actually mean. Signed-off-by: David Lamparter --- doc/developer/workflow.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/developer/workflow.rst b/doc/developer/workflow.rst index b6fde2b283..c703be62fc 100644 --- a/doc/developer/workflow.rst +++ b/doc/developer/workflow.rst @@ -1217,6 +1217,20 @@ it possible to use your apis in paths that involve ``const`` objects. If you encounter existing apis that *could* be ``const``, consider including changes in your own pull-request. +Help with specific warnings +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +FRR's configure script enables a whole batch of extra warnings, some of which +may not be obvious in how to fix. Here are some notes on specific warnings: + +* ``-Wstrict-prototypes``: you probably just forgot the ``void`` in a function + declaration with no parameters, i.e. ``static void foo() {...}`` rather than + ``static void foo(void) {...}``. + + Without the ``void``, in C, it's a function with *unspecified* parameters + (and varargs calling convention.) This is a notable difference to C++, where + the ``void`` is optional and an empty parameter list means no parameters. + .. _documentation: From 2554d1265c11257a79bfa541c5f0efcfdf097312 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 2 Sep 2021 12:19:59 +0200 Subject: [PATCH 7/7] build: make `sed` calls portable The `-i` option on sed isn't standard, and e.g. FreeBSD sed behaves different regarding the parameter. Avoid it. Signed-off-by: David Lamparter --- configure.ac | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index cf1c1e499c..917e791182 100644 --- a/configure.ac +++ b/configure.ac @@ -487,9 +487,12 @@ LT_INIT _LT_CONFIG_LIBTOOL([ patch -N -i "${srcdir}/m4/libtool-whole-archive.patch" libtool >&AS_MESSAGE_LOG_FD || \ AC_MSG_WARN([Could not patch libtool for static linking support. Loading modules into a statically linked daemon will fail.]) - sed -e 's%func_warning "relinking%true #\0%' -i libtool || true - sed -e 's%func_warning "remember to run%true #\0%' -i libtool || true - sed -e 's%func_warning ".*has not been installed in%true #\0%' -i libtool || true +dnl the -i option is not POSIX sed and the BSDs implement it differently +dnl cat'ing the output back instead of mv/cp keeps permissions on libtool intact + sed -e 's%func_warning "relinking%true #\0%' libtool > libtool.sed && cat libtool.sed > libtool + sed -e 's%func_warning "remember to run%true #\0%' libtool > libtool.sed && cat libtool.sed > libtool + sed -e 's%func_warning ".*has not been installed in%true #\0%' libtool > libtool.sed && cat libtool.sed > libtool + test -f libtool.sed && rm libtool.sed ]) if test "$enable_static_bin" = "yes"; then AC_LDFLAGS_EXEC="-static" @@ -2659,8 +2662,9 @@ if test "$enable_rpath" = "yes" ; then true else # See https://old-en.opensuse.org/openSUSE:Packaging_Guidelines#Removing_Rpath - sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool - sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool + sed -e 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool > libtool.sed && cat libtool.sed > libtool + sed -e 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool > libtool.sed && cat libtool.sed > libtool + test -f libtool.sed && rm libtool.sed fi echo "