Merge pull request #9563 from opensourcerouting/build-strict-prototypes

build: `-Wstrict-prototypes` (+ BSD `sed`)

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])
@@ -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
+dnl the -i option is not POSIX sed and the BSDs implement it differently
-  sed -e 's%func_warning "remember to run%true #\0%' -i libtool || true
+dnl cat'ing the output back instead of mv/cp keeps permissions on libtool intact
-  sed -e 's%func_warning ".*has not been installed in%true #\0%' -i libtool || true
+  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 -e 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool > libtool.sed && cat libtool.sed > libtool
-	sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' 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 "

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:
 

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);
 

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;
+static char *my_index(const char *str, int chr)
-int chr;
 {
 	while (*str) {
 		if (*str == chr)

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(       \
+	__attribute__((unused)) static inline void *                           \
-		int(*funcptr) arglist)                                         \
+		_hook_typecheck_##hookname(int(*funcptr) HOOK_VOIDIFY arglist) \
 	{                                                                      \
 		return (void *)funcptr;                                        \
 	}                                                                      \
-	__attribute__((unused)) static void *_hook_typecheck_arg_##hookname(   \
+	__attribute__((unused)) static inline void                             \
-		int(*funcptr) HOOK_ADDDEF arglist)                             \
+		*_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) {                                    \

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;
 

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);

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;
 

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;

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,

vtysh/vtysh.c

@@ -26,8 +26,12 @@
 #include <sys/resource.h>
 #include <sys/stat.h>
 
+/* readline carries some ancient definitions around */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstrict-prototypes"
 #include <readline/readline.h>
 #include <readline/history.h>
+#pragma GCC diagnostic pop
 
 #include <dirent.h>
 #include <stdio.h>

vtysh/vtysh_main.c

@@ -27,8 +27,12 @@
 #include <sys/file.h>
 #include <unistd.h>
 
+/* readline carries some ancient definitions around */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstrict-prototypes"
 #include <readline/readline.h>
 #include <readline/history.h>
+#pragma GCC diagnostic pop
 
 /*
  * The append_history function only appears in newer versions