Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
* [nfs-utils PATCH] Add printf format checking
@ 2019-09-06 16:33 Joshua Watt
  2019-09-06 20:24 ` [nfs-utils PATCH v2] " Joshua Watt
  0 siblings, 1 reply; 5+ messages in thread
From: Joshua Watt @ 2019-09-06 16:33 UTC (permalink / raw)
  To: linux-nfs; +Cc: Joshua Watt

Adds a configure time check for __attribute__((format)) and then uses it
to enforce checking the format of several printf-like functions. Several
invalid uses of format codes that were discovered have now been fixed.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 aclocal/ax_gcc_func_attribute.m4 | 238 +++++++++++++++++++++++++++++++
 configure.ac                     |   1 +
 support/include/xcommon.h        |  18 ++-
 support/include/xlog.h           |  20 ++-
 support/nfs/svc_create.c         |   2 +-
 support/nsm/rpc.c                |   2 +-
 utils/exportfs/exportfs.c        |   3 +
 utils/mountd/cache.c             |   3 +-
 utils/mountd/mountd.c            |   4 +-
 utils/nfsdcld/nfsdcld.c          |   2 +-
 utils/nfsdcld/sqlite.c           |   2 +-
 utils/nfsidmap/nfsidmap.c        |   8 +-
 utils/statd/rmtcall.c            |   2 +-
 utils/statd/statd.c              |   2 +-
 utils/statd/svc_run.c            |   2 +-
 15 files changed, 285 insertions(+), 24 deletions(-)
 create mode 100644 aclocal/ax_gcc_func_attribute.m4

diff --git a/aclocal/ax_gcc_func_attribute.m4 b/aclocal/ax_gcc_func_attribute.m4
new file mode 100644
index 00000000..098c9aad
--- /dev/null
+++ b/aclocal/ax_gcc_func_attribute.m4
@@ -0,0 +1,238 @@
+# ===========================================================================
+#  https://www.gnu.org/software/autoconf-archive/ax_gcc_func_attribute.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_GCC_FUNC_ATTRIBUTE(ATTRIBUTE)
+#
+# DESCRIPTION
+#
+#   This macro checks if the compiler supports one of GCC's function
+#   attributes; many other compilers also provide function attributes with
+#   the same syntax. Compiler warnings are used to detect supported
+#   attributes as unsupported ones are ignored by default so quieting
+#   warnings when using this macro will yield false positives.
+#
+#   The ATTRIBUTE parameter holds the name of the attribute to be checked.
+#
+#   If ATTRIBUTE is supported define HAVE_FUNC_ATTRIBUTE_<ATTRIBUTE>.
+#
+#   The macro caches its result in the ax_cv_have_func_attribute_<attribute>
+#   variable.
+#
+#   The macro currently supports the following function attributes:
+#
+#    alias
+#    aligned
+#    alloc_size
+#    always_inline
+#    artificial
+#    cold
+#    const
+#    constructor
+#    constructor_priority for constructor attribute with priority
+#    deprecated
+#    destructor
+#    dllexport
+#    dllimport
+#    error
+#    externally_visible
+#    fallthrough
+#    flatten
+#    format
+#    format_arg
+#    gnu_inline
+#    hot
+#    ifunc
+#    leaf
+#    malloc
+#    noclone
+#    noinline
+#    nonnull
+#    noreturn
+#    nothrow
+#    optimize
+#    pure
+#    sentinel
+#    sentinel_position
+#    unused
+#    used
+#    visibility
+#    warning
+#    warn_unused_result
+#    weak
+#    weakref
+#
+#   Unsupported function attributes will be tested with a prototype
+#   returning an int and not accepting any arguments and the result of the
+#   check might be wrong or meaningless so use with care.
+#
+# LICENSE
+#
+#   Copyright (c) 2013 Gabriele Svelto <gabriele.svelto@gmail.com>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved.  This file is offered as-is, without any
+#   warranty.
+
+#serial 9
+
+AC_DEFUN([AX_GCC_FUNC_ATTRIBUTE], [
+    AS_VAR_PUSHDEF([ac_var], [ax_cv_have_func_attribute_$1])
+
+    AC_CACHE_CHECK([for __attribute__(($1))], [ac_var], [
+        AC_LINK_IFELSE([AC_LANG_PROGRAM([
+            m4_case([$1],
+                [alias], [
+                    int foo( void ) { return 0; }
+                    int bar( void ) __attribute__(($1("foo")));
+                ],
+                [aligned], [
+                    int foo( void ) __attribute__(($1(32)));
+                ],
+                [alloc_size], [
+                    void *foo(int a) __attribute__(($1(1)));
+                ],
+                [always_inline], [
+                    inline __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [artificial], [
+                    inline __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [cold], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [const], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [constructor_priority], [
+                    int foo( void ) __attribute__((__constructor__(65535/2)));
+                ],
+                [constructor], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [deprecated], [
+                    int foo( void ) __attribute__(($1("")));
+                ],
+                [destructor], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [dllexport], [
+                    __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [dllimport], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [error], [
+                    int foo( void ) __attribute__(($1("")));
+                ],
+                [externally_visible], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [fallthrough], [
+                    int foo( void ) {switch (0) { case 1: __attribute__(($1)); case 2: break ; }};
+                ],
+                [flatten], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [format], [
+                    int foo(const char *p, ...) __attribute__(($1(printf, 1, 2)));
+                ],
+                [format_arg], [
+                    char *foo(const char *p) __attribute__(($1(1)));
+                ],
+                [gnu_inline], [
+                    inline __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [hot], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [ifunc], [
+                    int my_foo( void ) { return 0; }
+                    static int (*resolve_foo(void))(void) { return my_foo; }
+                    int foo( void ) __attribute__(($1("resolve_foo")));
+                ],
+                [leaf], [
+                    __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [malloc], [
+                    void *foo( void ) __attribute__(($1));
+                ],
+                [noclone], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [noinline], [
+                    __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [nonnull], [
+                    int foo(char *p) __attribute__(($1(1)));
+                ],
+                [noreturn], [
+                    void foo( void ) __attribute__(($1));
+                ],
+                [nothrow], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [optimize], [
+                    __attribute__(($1(3))) int foo( void ) { return 0; }
+                ],
+                [pure], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [sentinel], [
+                    int foo(void *p, ...) __attribute__(($1));
+                ],
+                [sentinel_position], [
+                    int foo(void *p, ...) __attribute__(($1(1)));
+                ],
+                [returns_nonnull], [
+                    void *foo( void ) __attribute__(($1));
+                ],
+                [unused], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [used], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [visibility], [
+                    int foo_def( void ) __attribute__(($1("default")));
+                    int foo_hid( void ) __attribute__(($1("hidden")));
+                    int foo_int( void ) __attribute__(($1("internal")));
+                    int foo_pro( void ) __attribute__(($1("protected")));
+                ],
+                [warning], [
+                    int foo( void ) __attribute__(($1("")));
+                ],
+                [warn_unused_result], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [weak], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [weakref], [
+                    static int foo( void ) { return 0; }
+                    static int bar( void ) __attribute__(($1("foo")));
+                ],
+                [
+                 m4_warn([syntax], [Unsupported attribute $1, the test may fail])
+                 int foo( void ) __attribute__(($1));
+                ]
+            )], [])
+            ],
+            dnl GCC doesn't exit with an error if an unknown attribute is
+            dnl provided but only outputs a warning, so accept the attribute
+            dnl only if no warning were issued.
+            [AS_IF([test -s conftest.err],
+                [AS_VAR_SET([ac_var], [no])],
+                [AS_VAR_SET([ac_var], [yes])])],
+            [AS_VAR_SET([ac_var], [no])])
+    ])
+
+    AS_IF([test yes = AS_VAR_GET([ac_var])],
+        [AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_FUNC_ATTRIBUTE_$1), 1,
+            [Define to 1 if the system has the `$1' function attribute])], [])
+
+    AS_VAR_POPDEF([ac_var])
+])
diff --git a/configure.ac b/configure.ac
index 37096944..639199a9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -619,6 +619,7 @@ CHECK_CCSUPPORT([-Werror=format-overflow=2], [flg1])
 CHECK_CCSUPPORT([-Werror=int-conversion], [flg2])
 CHECK_CCSUPPORT([-Werror=incompatible-pointer-types], [flg3])
 CHECK_CCSUPPORT([-Werror=misleading-indentation], [flg4])
+AX_GCC_FUNC_ATTRIBUTE([format])
 
 AC_SUBST([AM_CFLAGS], ["$my_am_cflags $flg1 $flg2 $flg3 $flg4"])
 
diff --git a/support/include/xcommon.h b/support/include/xcommon.h
index 23c9a135..30b0403b 100644
--- a/support/include/xcommon.h
+++ b/support/include/xcommon.h
@@ -9,6 +9,10 @@
 #ifndef _XMALLOC_H
 #define _MALLOC_H
 
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
 #include <sys/types.h>
 #include <fcntl.h>
 #include <limits.h>
@@ -25,9 +29,15 @@
 
 #define streq(s, t)	(strcmp ((s), (t)) == 0)
 
-/* Functions in sundries.c that are used in mount.c and umount.c  */ 
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+#define X_FORMAT(_x) __attribute__((__format__ _x))
+#else
+#define X_FORMAT(_x)
+#endif
+
+/* Functions in sundries.c that are used in mount.c and umount.c  */
 char *canonicalize (const char *path);
-void nfs_error (const char *fmt, ...);
+void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
 void *xmalloc (size_t size);
 void *xrealloc(void *p, size_t size);
 void xfree(void *);
@@ -36,9 +46,9 @@ char *xstrndup (const char *s, int n);
 char *xstrconcat2 (const char *, const char *);
 char *xstrconcat3 (const char *, const char *, const char *);
 char *xstrconcat4 (const char *, const char *, const char *, const char *);
-void die (int errcode, const char *fmt, ...);
+void die (int errcode, const char *fmt, ...) X_FORMAT((printf, 2, 3));
 
-extern void die(int err, const char *fmt, ...);
+extern void die(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
 extern void (*at_die)(void);
 
 /* exit status - bits below are ORed */
diff --git a/support/include/xlog.h b/support/include/xlog.h
index a11463ed..32ff5a1b 100644
--- a/support/include/xlog.h
+++ b/support/include/xlog.h
@@ -7,6 +7,10 @@
 #ifndef XLOG_H
 #define XLOG_H
 
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
 #include <stdarg.h>
 
 /* These are logged always. L_FATAL also does exit(1) */
@@ -35,6 +39,12 @@ struct xlog_debugfac {
 	int		df_fac;
 };
 
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
+#else
+#define XLOG_FORMAT(_x)
+#endif
+
 extern int export_errno;
 void			xlog_open(char *progname);
 void			xlog_stderr(int on);
@@ -43,10 +53,10 @@ void			xlog_config(int fac, int on);
 void			xlog_sconfig(char *, int on);
 void			xlog_from_conffile(char *);
 int			xlog_enabled(int fac);
-void			xlog(int fac, const char *fmt, ...);
-void			xlog_warn(const char *fmt, ...);
-void			xlog_err(const char *fmt, ...);
-void			xlog_errno(int err, const char *fmt, ...);
-void			xlog_backend(int fac, const char *fmt, va_list args);
+void			xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
+void			xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
+void			xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
+void			xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
+void			xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
 
 #endif /* XLOG_H */
diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index 4e14430d..976c2d29 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -184,7 +184,7 @@ svc_create_sock(const struct sockaddr *sap, socklen_t salen,
 		type = SOCK_STREAM;
 		break;
 	default:
-		xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %u",
+		xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %lu",
 			__func__, nconf->nc_semantics);
 		return -1;
 	}
diff --git a/support/nsm/rpc.c b/support/nsm/rpc.c
index ae49006c..08b4746f 100644
--- a/support/nsm/rpc.c
+++ b/support/nsm/rpc.c
@@ -182,7 +182,7 @@ nsm_xmit_getport(const int sock, const struct sockaddr_in *sin,
 	uint32_t xid;
 	XDR xdr;
 
-	xlog(D_CALL, "Sending PMAP_GETPORT for %u, %u, udp", program, version);
+	xlog(D_CALL, "Sending PMAP_GETPORT for %lu, %lu, udp", program, version);
 
 	nsm_init_xdrmem(msgbuf, NSM_MAXMSGSIZE, &xdr);
 	xid = nsm_init_rpc_header(PMAPPROG, PMAPVERS,
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 5cca4175..a04a7898 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -651,6 +651,9 @@ out:
 	return result;
 }
 
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+__attribute__((format (printf, 2, 3)))
+#endif
 static char
 dumpopt(char c, char *fmt, ...)
 {
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index e25a4337..3861f84a 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -987,8 +987,7 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
 			} else if (found_type == i && found->m_warned == 0) {
 				xlog(L_WARNING, "%s exported to both %s and %s, "
 				     "arbitrarily choosing options from first",
-				     path, found->m_client->m_hostname, exp->m_client->m_hostname,
-				     dom);
+				     path, found->m_client->m_hostname, exp->m_client->m_hostname);
 				found->m_warned = 1;
 			}
 		}
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 33571ecb..66366434 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -210,10 +210,10 @@ killer (int sig)
 }
 
 static void
-sig_hup (int sig)
+sig_hup (int UNUSED(sig))
 {
 	/* don't exit on SIGHUP */
-	xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n", sig);
+	xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n");
 	return;
 }
 
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index cbf71fc6..7e894e49 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -212,7 +212,7 @@ cld_inotify_cb(int UNUSED(fd), short which, void *data)
 	default:
 		/* anything else is fatal */
 		xlog(L_FATAL, "%s: unable to open new pipe (%d). Aborting.",
-			ret, __func__);
+			__func__, ret);
 		exit(ret);
 	}
 
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index fa81df87..afa63499 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -473,7 +473,7 @@ sqlite_fix_table_name(const char *name)
 	}
 	ret = sqlite3_exec(dbh, (const char *)buf, NULL, NULL, &err);
 	if (ret != SQLITE_OK) {
-		xlog(L_ERROR, "Unable to fix table for epoch %d: %s",
+		xlog(L_ERROR, "Unable to fix table for epoch %"PRIu64": %s",
 		     val, err);
 		goto out;
 	}
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index fc00da7a..cf7f65e9 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -18,7 +18,7 @@
 #include "xcommon.h"
 
 int verbose = 0;
-char *usage = "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]";
+#define USAGE "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]"
 
 #define MAX_ID_LEN   11
 #define IDMAP_NAMESZ 128
@@ -403,7 +403,7 @@ int main(int argc, char **argv)
 			break;
 		case 'h':
 		default:
-			xlog_warn(usage, progname);
+			xlog_warn(USAGE, progname);
 			exit(opt == 'h' ? 0 : 1);
 		}
 	}
@@ -435,7 +435,7 @@ int main(int argc, char **argv)
 	xlog_stderr(verbose);
 	if ((argc - optind) != 2) {
 		xlog_warn("Bad arg count. Check /etc/request-key.conf");
-		xlog_warn(usage, progname);
+		xlog_warn(USAGE, progname);
 		return EXIT_FAILURE;
 	}
 
@@ -453,7 +453,7 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 	if (verbose) {
-		xlog_warn("key: 0x%lx type: %s value: %s timeout %ld",
+		xlog_warn("key: 0x%x type: %s value: %s timeout %d",
 			key, type, value, timeout);
 	}
 
diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
index c4f6364f..5b261480 100644
--- a/utils/statd/rmtcall.c
+++ b/utils/statd/rmtcall.c
@@ -247,7 +247,7 @@ process_reply(FD_SET_TYPE *rfds)
 		xlog_warn("%s: service %d not registered on localhost",
 			__func__, NL_MY_PROG(lp));
 	} else {
-		xlog(D_GENERAL, "%s: Callback to %s (for %d) succeeded",
+		xlog(D_GENERAL, "%s: Callback to %s (for %s) succeeded",
 			__func__, NL_MY_NAME(lp), NL_MON_NAME(lp));
 	}
 	nlist_free(&notify, lp);
diff --git a/utils/statd/statd.c b/utils/statd/statd.c
index 14673800..8eef2ff2 100644
--- a/utils/statd/statd.c
+++ b/utils/statd/statd.c
@@ -136,7 +136,7 @@ static void log_modes(void)
 	strcat(buf, "TI-RPC ");
 #endif
 
-	xlog_warn(buf);
+	xlog_warn("%s", buf);
 }
 
 /*
diff --git a/utils/statd/svc_run.c b/utils/statd/svc_run.c
index d1dbd74a..ec61cd11 100644
--- a/utils/statd/svc_run.c
+++ b/utils/statd/svc_run.c
@@ -104,7 +104,7 @@ my_svc_run(int sockfd)
 
 			tv.tv_sec  = NL_WHEN(notify) - now;
 			tv.tv_usec = 0;
-			xlog(D_GENERAL, "Waiting for reply... (timeo %d)",
+			xlog(D_GENERAL, "Waiting for reply... (timeo %jd)",
 							tv.tv_sec);
 			selret = select(FD_SETSIZE, &readfds,
 				(void *) 0, (void *) 0, &tv);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [nfs-utils PATCH v2] Add printf format checking
  2019-09-06 16:33 [nfs-utils PATCH] Add printf format checking Joshua Watt
@ 2019-09-06 20:24 ` " Joshua Watt
  2019-09-16 18:33   ` Joshua Watt
  0 siblings, 1 reply; 5+ messages in thread
From: Joshua Watt @ 2019-09-06 20:24 UTC (permalink / raw)
  To: linux-nfs; +Cc: Joshua Watt

Adds a configure time check for __attribute__((format)) and then uses it
to enforce checking the format of several printf-like functions. Several
invalid uses of format codes that were discovered have now been fixed.

V2: Fix use of "%jd" format code on an argument that wasn't intmax_t

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 aclocal/ax_gcc_func_attribute.m4 | 238 +++++++++++++++++++++++++++++++
 configure.ac                     |   1 +
 support/include/xcommon.h        |  18 ++-
 support/include/xlog.h           |  20 ++-
 support/nfs/svc_create.c         |   2 +-
 support/nsm/rpc.c                |   2 +-
 utils/exportfs/exportfs.c        |   3 +
 utils/mountd/cache.c             |   3 +-
 utils/mountd/mountd.c            |   4 +-
 utils/nfsdcld/nfsdcld.c          |   2 +-
 utils/nfsdcld/sqlite.c           |   2 +-
 utils/nfsidmap/nfsidmap.c        |   8 +-
 utils/statd/rmtcall.c            |   2 +-
 utils/statd/statd.c              |   2 +-
 utils/statd/svc_run.c            |   5 +-
 15 files changed, 287 insertions(+), 25 deletions(-)
 create mode 100644 aclocal/ax_gcc_func_attribute.m4

diff --git a/aclocal/ax_gcc_func_attribute.m4 b/aclocal/ax_gcc_func_attribute.m4
new file mode 100644
index 00000000..098c9aad
--- /dev/null
+++ b/aclocal/ax_gcc_func_attribute.m4
@@ -0,0 +1,238 @@
+# ===========================================================================
+#  https://www.gnu.org/software/autoconf-archive/ax_gcc_func_attribute.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_GCC_FUNC_ATTRIBUTE(ATTRIBUTE)
+#
+# DESCRIPTION
+#
+#   This macro checks if the compiler supports one of GCC's function
+#   attributes; many other compilers also provide function attributes with
+#   the same syntax. Compiler warnings are used to detect supported
+#   attributes as unsupported ones are ignored by default so quieting
+#   warnings when using this macro will yield false positives.
+#
+#   The ATTRIBUTE parameter holds the name of the attribute to be checked.
+#
+#   If ATTRIBUTE is supported define HAVE_FUNC_ATTRIBUTE_<ATTRIBUTE>.
+#
+#   The macro caches its result in the ax_cv_have_func_attribute_<attribute>
+#   variable.
+#
+#   The macro currently supports the following function attributes:
+#
+#    alias
+#    aligned
+#    alloc_size
+#    always_inline
+#    artificial
+#    cold
+#    const
+#    constructor
+#    constructor_priority for constructor attribute with priority
+#    deprecated
+#    destructor
+#    dllexport
+#    dllimport
+#    error
+#    externally_visible
+#    fallthrough
+#    flatten
+#    format
+#    format_arg
+#    gnu_inline
+#    hot
+#    ifunc
+#    leaf
+#    malloc
+#    noclone
+#    noinline
+#    nonnull
+#    noreturn
+#    nothrow
+#    optimize
+#    pure
+#    sentinel
+#    sentinel_position
+#    unused
+#    used
+#    visibility
+#    warning
+#    warn_unused_result
+#    weak
+#    weakref
+#
+#   Unsupported function attributes will be tested with a prototype
+#   returning an int and not accepting any arguments and the result of the
+#   check might be wrong or meaningless so use with care.
+#
+# LICENSE
+#
+#   Copyright (c) 2013 Gabriele Svelto <gabriele.svelto@gmail.com>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved.  This file is offered as-is, without any
+#   warranty.
+
+#serial 9
+
+AC_DEFUN([AX_GCC_FUNC_ATTRIBUTE], [
+    AS_VAR_PUSHDEF([ac_var], [ax_cv_have_func_attribute_$1])
+
+    AC_CACHE_CHECK([for __attribute__(($1))], [ac_var], [
+        AC_LINK_IFELSE([AC_LANG_PROGRAM([
+            m4_case([$1],
+                [alias], [
+                    int foo( void ) { return 0; }
+                    int bar( void ) __attribute__(($1("foo")));
+                ],
+                [aligned], [
+                    int foo( void ) __attribute__(($1(32)));
+                ],
+                [alloc_size], [
+                    void *foo(int a) __attribute__(($1(1)));
+                ],
+                [always_inline], [
+                    inline __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [artificial], [
+                    inline __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [cold], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [const], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [constructor_priority], [
+                    int foo( void ) __attribute__((__constructor__(65535/2)));
+                ],
+                [constructor], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [deprecated], [
+                    int foo( void ) __attribute__(($1("")));
+                ],
+                [destructor], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [dllexport], [
+                    __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [dllimport], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [error], [
+                    int foo( void ) __attribute__(($1("")));
+                ],
+                [externally_visible], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [fallthrough], [
+                    int foo( void ) {switch (0) { case 1: __attribute__(($1)); case 2: break ; }};
+                ],
+                [flatten], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [format], [
+                    int foo(const char *p, ...) __attribute__(($1(printf, 1, 2)));
+                ],
+                [format_arg], [
+                    char *foo(const char *p) __attribute__(($1(1)));
+                ],
+                [gnu_inline], [
+                    inline __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [hot], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [ifunc], [
+                    int my_foo( void ) { return 0; }
+                    static int (*resolve_foo(void))(void) { return my_foo; }
+                    int foo( void ) __attribute__(($1("resolve_foo")));
+                ],
+                [leaf], [
+                    __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [malloc], [
+                    void *foo( void ) __attribute__(($1));
+                ],
+                [noclone], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [noinline], [
+                    __attribute__(($1)) int foo( void ) { return 0; }
+                ],
+                [nonnull], [
+                    int foo(char *p) __attribute__(($1(1)));
+                ],
+                [noreturn], [
+                    void foo( void ) __attribute__(($1));
+                ],
+                [nothrow], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [optimize], [
+                    __attribute__(($1(3))) int foo( void ) { return 0; }
+                ],
+                [pure], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [sentinel], [
+                    int foo(void *p, ...) __attribute__(($1));
+                ],
+                [sentinel_position], [
+                    int foo(void *p, ...) __attribute__(($1(1)));
+                ],
+                [returns_nonnull], [
+                    void *foo( void ) __attribute__(($1));
+                ],
+                [unused], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [used], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [visibility], [
+                    int foo_def( void ) __attribute__(($1("default")));
+                    int foo_hid( void ) __attribute__(($1("hidden")));
+                    int foo_int( void ) __attribute__(($1("internal")));
+                    int foo_pro( void ) __attribute__(($1("protected")));
+                ],
+                [warning], [
+                    int foo( void ) __attribute__(($1("")));
+                ],
+                [warn_unused_result], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [weak], [
+                    int foo( void ) __attribute__(($1));
+                ],
+                [weakref], [
+                    static int foo( void ) { return 0; }
+                    static int bar( void ) __attribute__(($1("foo")));
+                ],
+                [
+                 m4_warn([syntax], [Unsupported attribute $1, the test may fail])
+                 int foo( void ) __attribute__(($1));
+                ]
+            )], [])
+            ],
+            dnl GCC doesn't exit with an error if an unknown attribute is
+            dnl provided but only outputs a warning, so accept the attribute
+            dnl only if no warning were issued.
+            [AS_IF([test -s conftest.err],
+                [AS_VAR_SET([ac_var], [no])],
+                [AS_VAR_SET([ac_var], [yes])])],
+            [AS_VAR_SET([ac_var], [no])])
+    ])
+
+    AS_IF([test yes = AS_VAR_GET([ac_var])],
+        [AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_FUNC_ATTRIBUTE_$1), 1,
+            [Define to 1 if the system has the `$1' function attribute])], [])
+
+    AS_VAR_POPDEF([ac_var])
+])
diff --git a/configure.ac b/configure.ac
index 37096944..639199a9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -619,6 +619,7 @@ CHECK_CCSUPPORT([-Werror=format-overflow=2], [flg1])
 CHECK_CCSUPPORT([-Werror=int-conversion], [flg2])
 CHECK_CCSUPPORT([-Werror=incompatible-pointer-types], [flg3])
 CHECK_CCSUPPORT([-Werror=misleading-indentation], [flg4])
+AX_GCC_FUNC_ATTRIBUTE([format])
 
 AC_SUBST([AM_CFLAGS], ["$my_am_cflags $flg1 $flg2 $flg3 $flg4"])
 
diff --git a/support/include/xcommon.h b/support/include/xcommon.h
index 23c9a135..30b0403b 100644
--- a/support/include/xcommon.h
+++ b/support/include/xcommon.h
@@ -9,6 +9,10 @@
 #ifndef _XMALLOC_H
 #define _MALLOC_H
 
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
 #include <sys/types.h>
 #include <fcntl.h>
 #include <limits.h>
@@ -25,9 +29,15 @@
 
 #define streq(s, t)	(strcmp ((s), (t)) == 0)
 
-/* Functions in sundries.c that are used in mount.c and umount.c  */ 
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+#define X_FORMAT(_x) __attribute__((__format__ _x))
+#else
+#define X_FORMAT(_x)
+#endif
+
+/* Functions in sundries.c that are used in mount.c and umount.c  */
 char *canonicalize (const char *path);
-void nfs_error (const char *fmt, ...);
+void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
 void *xmalloc (size_t size);
 void *xrealloc(void *p, size_t size);
 void xfree(void *);
@@ -36,9 +46,9 @@ char *xstrndup (const char *s, int n);
 char *xstrconcat2 (const char *, const char *);
 char *xstrconcat3 (const char *, const char *, const char *);
 char *xstrconcat4 (const char *, const char *, const char *, const char *);
-void die (int errcode, const char *fmt, ...);
+void die (int errcode, const char *fmt, ...) X_FORMAT((printf, 2, 3));
 
-extern void die(int err, const char *fmt, ...);
+extern void die(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
 extern void (*at_die)(void);
 
 /* exit status - bits below are ORed */
diff --git a/support/include/xlog.h b/support/include/xlog.h
index a11463ed..32ff5a1b 100644
--- a/support/include/xlog.h
+++ b/support/include/xlog.h
@@ -7,6 +7,10 @@
 #ifndef XLOG_H
 #define XLOG_H
 
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
 #include <stdarg.h>
 
 /* These are logged always. L_FATAL also does exit(1) */
@@ -35,6 +39,12 @@ struct xlog_debugfac {
 	int		df_fac;
 };
 
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
+#else
+#define XLOG_FORMAT(_x)
+#endif
+
 extern int export_errno;
 void			xlog_open(char *progname);
 void			xlog_stderr(int on);
@@ -43,10 +53,10 @@ void			xlog_config(int fac, int on);
 void			xlog_sconfig(char *, int on);
 void			xlog_from_conffile(char *);
 int			xlog_enabled(int fac);
-void			xlog(int fac, const char *fmt, ...);
-void			xlog_warn(const char *fmt, ...);
-void			xlog_err(const char *fmt, ...);
-void			xlog_errno(int err, const char *fmt, ...);
-void			xlog_backend(int fac, const char *fmt, va_list args);
+void			xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
+void			xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
+void			xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
+void			xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
+void			xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
 
 #endif /* XLOG_H */
diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
index 4e14430d..976c2d29 100644
--- a/support/nfs/svc_create.c
+++ b/support/nfs/svc_create.c
@@ -184,7 +184,7 @@ svc_create_sock(const struct sockaddr *sap, socklen_t salen,
 		type = SOCK_STREAM;
 		break;
 	default:
-		xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %u",
+		xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %lu",
 			__func__, nconf->nc_semantics);
 		return -1;
 	}
diff --git a/support/nsm/rpc.c b/support/nsm/rpc.c
index ae49006c..08b4746f 100644
--- a/support/nsm/rpc.c
+++ b/support/nsm/rpc.c
@@ -182,7 +182,7 @@ nsm_xmit_getport(const int sock, const struct sockaddr_in *sin,
 	uint32_t xid;
 	XDR xdr;
 
-	xlog(D_CALL, "Sending PMAP_GETPORT for %u, %u, udp", program, version);
+	xlog(D_CALL, "Sending PMAP_GETPORT for %lu, %lu, udp", program, version);
 
 	nsm_init_xdrmem(msgbuf, NSM_MAXMSGSIZE, &xdr);
 	xid = nsm_init_rpc_header(PMAPPROG, PMAPVERS,
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index 5cca4175..a04a7898 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -651,6 +651,9 @@ out:
 	return result;
 }
 
+#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
+__attribute__((format (printf, 2, 3)))
+#endif
 static char
 dumpopt(char c, char *fmt, ...)
 {
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index e25a4337..3861f84a 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -987,8 +987,7 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
 			} else if (found_type == i && found->m_warned == 0) {
 				xlog(L_WARNING, "%s exported to both %s and %s, "
 				     "arbitrarily choosing options from first",
-				     path, found->m_client->m_hostname, exp->m_client->m_hostname,
-				     dom);
+				     path, found->m_client->m_hostname, exp->m_client->m_hostname);
 				found->m_warned = 1;
 			}
 		}
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 33571ecb..66366434 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -210,10 +210,10 @@ killer (int sig)
 }
 
 static void
-sig_hup (int sig)
+sig_hup (int UNUSED(sig))
 {
 	/* don't exit on SIGHUP */
-	xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n", sig);
+	xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n");
 	return;
 }
 
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
index cbf71fc6..7e894e49 100644
--- a/utils/nfsdcld/nfsdcld.c
+++ b/utils/nfsdcld/nfsdcld.c
@@ -212,7 +212,7 @@ cld_inotify_cb(int UNUSED(fd), short which, void *data)
 	default:
 		/* anything else is fatal */
 		xlog(L_FATAL, "%s: unable to open new pipe (%d). Aborting.",
-			ret, __func__);
+			__func__, ret);
 		exit(ret);
 	}
 
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
index fa81df87..afa63499 100644
--- a/utils/nfsdcld/sqlite.c
+++ b/utils/nfsdcld/sqlite.c
@@ -473,7 +473,7 @@ sqlite_fix_table_name(const char *name)
 	}
 	ret = sqlite3_exec(dbh, (const char *)buf, NULL, NULL, &err);
 	if (ret != SQLITE_OK) {
-		xlog(L_ERROR, "Unable to fix table for epoch %d: %s",
+		xlog(L_ERROR, "Unable to fix table for epoch %"PRIu64": %s",
 		     val, err);
 		goto out;
 	}
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index fc00da7a..cf7f65e9 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -18,7 +18,7 @@
 #include "xcommon.h"
 
 int verbose = 0;
-char *usage = "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]";
+#define USAGE "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]"
 
 #define MAX_ID_LEN   11
 #define IDMAP_NAMESZ 128
@@ -403,7 +403,7 @@ int main(int argc, char **argv)
 			break;
 		case 'h':
 		default:
-			xlog_warn(usage, progname);
+			xlog_warn(USAGE, progname);
 			exit(opt == 'h' ? 0 : 1);
 		}
 	}
@@ -435,7 +435,7 @@ int main(int argc, char **argv)
 	xlog_stderr(verbose);
 	if ((argc - optind) != 2) {
 		xlog_warn("Bad arg count. Check /etc/request-key.conf");
-		xlog_warn(usage, progname);
+		xlog_warn(USAGE, progname);
 		return EXIT_FAILURE;
 	}
 
@@ -453,7 +453,7 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 	if (verbose) {
-		xlog_warn("key: 0x%lx type: %s value: %s timeout %ld",
+		xlog_warn("key: 0x%x type: %s value: %s timeout %d",
 			key, type, value, timeout);
 	}
 
diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
index c4f6364f..5b261480 100644
--- a/utils/statd/rmtcall.c
+++ b/utils/statd/rmtcall.c
@@ -247,7 +247,7 @@ process_reply(FD_SET_TYPE *rfds)
 		xlog_warn("%s: service %d not registered on localhost",
 			__func__, NL_MY_PROG(lp));
 	} else {
-		xlog(D_GENERAL, "%s: Callback to %s (for %d) succeeded",
+		xlog(D_GENERAL, "%s: Callback to %s (for %s) succeeded",
 			__func__, NL_MY_NAME(lp), NL_MON_NAME(lp));
 	}
 	nlist_free(&notify, lp);
diff --git a/utils/statd/statd.c b/utils/statd/statd.c
index 14673800..8eef2ff2 100644
--- a/utils/statd/statd.c
+++ b/utils/statd/statd.c
@@ -136,7 +136,7 @@ static void log_modes(void)
 	strcat(buf, "TI-RPC ");
 #endif
 
-	xlog_warn(buf);
+	xlog_warn("%s", buf);
 }
 
 /*
diff --git a/utils/statd/svc_run.c b/utils/statd/svc_run.c
index d1dbd74a..e343c768 100644
--- a/utils/statd/svc_run.c
+++ b/utils/statd/svc_run.c
@@ -53,6 +53,7 @@
 
 #include <errno.h>
 #include <time.h>
+#include <inttypes.h>
 #include "statd.h"
 #include "notlist.h"
 
@@ -104,8 +105,8 @@ my_svc_run(int sockfd)
 
 			tv.tv_sec  = NL_WHEN(notify) - now;
 			tv.tv_usec = 0;
-			xlog(D_GENERAL, "Waiting for reply... (timeo %d)",
-							tv.tv_sec);
+			xlog(D_GENERAL, "Waiting for reply... (timeo %jd)",
+							(intmax_t)tv.tv_sec);
 			selret = select(FD_SETSIZE, &readfds,
 				(void *) 0, (void *) 0, &tv);
 		} else {
-- 
2.21.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [nfs-utils PATCH v2] Add printf format checking
  2019-09-06 20:24 ` [nfs-utils PATCH v2] " Joshua Watt
@ 2019-09-16 18:33   ` Joshua Watt
  2019-09-20 14:05     ` Steve Dickson
  0 siblings, 1 reply; 5+ messages in thread
From: Joshua Watt @ 2019-09-16 18:33 UTC (permalink / raw)
  To: linux-nfs

On 9/6/19 3:24 PM, Joshua Watt wrote:
> Adds a configure time check for __attribute__((format)) and then uses it
> to enforce checking the format of several printf-like functions. Several
> invalid uses of format codes that were discovered have now been fixed.
>
> V2: Fix use of "%jd" format code on an argument that wasn't intmax_t

ping?

>
> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> ---
>   aclocal/ax_gcc_func_attribute.m4 | 238 +++++++++++++++++++++++++++++++
>   configure.ac                     |   1 +
>   support/include/xcommon.h        |  18 ++-
>   support/include/xlog.h           |  20 ++-
>   support/nfs/svc_create.c         |   2 +-
>   support/nsm/rpc.c                |   2 +-
>   utils/exportfs/exportfs.c        |   3 +
>   utils/mountd/cache.c             |   3 +-
>   utils/mountd/mountd.c            |   4 +-
>   utils/nfsdcld/nfsdcld.c          |   2 +-
>   utils/nfsdcld/sqlite.c           |   2 +-
>   utils/nfsidmap/nfsidmap.c        |   8 +-
>   utils/statd/rmtcall.c            |   2 +-
>   utils/statd/statd.c              |   2 +-
>   utils/statd/svc_run.c            |   5 +-
>   15 files changed, 287 insertions(+), 25 deletions(-)
>   create mode 100644 aclocal/ax_gcc_func_attribute.m4
>
> diff --git a/aclocal/ax_gcc_func_attribute.m4 b/aclocal/ax_gcc_func_attribute.m4
> new file mode 100644
> index 00000000..098c9aad
> --- /dev/null
> +++ b/aclocal/ax_gcc_func_attribute.m4
> @@ -0,0 +1,238 @@
> +# ===========================================================================
> +#  https://www.gnu.org/software/autoconf-archive/ax_gcc_func_attribute.html
> +# ===========================================================================
> +#
> +# SYNOPSIS
> +#
> +#   AX_GCC_FUNC_ATTRIBUTE(ATTRIBUTE)
> +#
> +# DESCRIPTION
> +#
> +#   This macro checks if the compiler supports one of GCC's function
> +#   attributes; many other compilers also provide function attributes with
> +#   the same syntax. Compiler warnings are used to detect supported
> +#   attributes as unsupported ones are ignored by default so quieting
> +#   warnings when using this macro will yield false positives.
> +#
> +#   The ATTRIBUTE parameter holds the name of the attribute to be checked.
> +#
> +#   If ATTRIBUTE is supported define HAVE_FUNC_ATTRIBUTE_<ATTRIBUTE>.
> +#
> +#   The macro caches its result in the ax_cv_have_func_attribute_<attribute>
> +#   variable.
> +#
> +#   The macro currently supports the following function attributes:
> +#
> +#    alias
> +#    aligned
> +#    alloc_size
> +#    always_inline
> +#    artificial
> +#    cold
> +#    const
> +#    constructor
> +#    constructor_priority for constructor attribute with priority
> +#    deprecated
> +#    destructor
> +#    dllexport
> +#    dllimport
> +#    error
> +#    externally_visible
> +#    fallthrough
> +#    flatten
> +#    format
> +#    format_arg
> +#    gnu_inline
> +#    hot
> +#    ifunc
> +#    leaf
> +#    malloc
> +#    noclone
> +#    noinline
> +#    nonnull
> +#    noreturn
> +#    nothrow
> +#    optimize
> +#    pure
> +#    sentinel
> +#    sentinel_position
> +#    unused
> +#    used
> +#    visibility
> +#    warning
> +#    warn_unused_result
> +#    weak
> +#    weakref
> +#
> +#   Unsupported function attributes will be tested with a prototype
> +#   returning an int and not accepting any arguments and the result of the
> +#   check might be wrong or meaningless so use with care.
> +#
> +# LICENSE
> +#
> +#   Copyright (c) 2013 Gabriele Svelto <gabriele.svelto@gmail.com>
> +#
> +#   Copying and distribution of this file, with or without modification, are
> +#   permitted in any medium without royalty provided the copyright notice
> +#   and this notice are preserved.  This file is offered as-is, without any
> +#   warranty.
> +
> +#serial 9
> +
> +AC_DEFUN([AX_GCC_FUNC_ATTRIBUTE], [
> +    AS_VAR_PUSHDEF([ac_var], [ax_cv_have_func_attribute_$1])
> +
> +    AC_CACHE_CHECK([for __attribute__(($1))], [ac_var], [
> +        AC_LINK_IFELSE([AC_LANG_PROGRAM([
> +            m4_case([$1],
> +                [alias], [
> +                    int foo( void ) { return 0; }
> +                    int bar( void ) __attribute__(($1("foo")));
> +                ],
> +                [aligned], [
> +                    int foo( void ) __attribute__(($1(32)));
> +                ],
> +                [alloc_size], [
> +                    void *foo(int a) __attribute__(($1(1)));
> +                ],
> +                [always_inline], [
> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
> +                ],
> +                [artificial], [
> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
> +                ],
> +                [cold], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [const], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [constructor_priority], [
> +                    int foo( void ) __attribute__((__constructor__(65535/2)));
> +                ],
> +                [constructor], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [deprecated], [
> +                    int foo( void ) __attribute__(($1("")));
> +                ],
> +                [destructor], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [dllexport], [
> +                    __attribute__(($1)) int foo( void ) { return 0; }
> +                ],
> +                [dllimport], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [error], [
> +                    int foo( void ) __attribute__(($1("")));
> +                ],
> +                [externally_visible], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [fallthrough], [
> +                    int foo( void ) {switch (0) { case 1: __attribute__(($1)); case 2: break ; }};
> +                ],
> +                [flatten], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [format], [
> +                    int foo(const char *p, ...) __attribute__(($1(printf, 1, 2)));
> +                ],
> +                [format_arg], [
> +                    char *foo(const char *p) __attribute__(($1(1)));
> +                ],
> +                [gnu_inline], [
> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
> +                ],
> +                [hot], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [ifunc], [
> +                    int my_foo( void ) { return 0; }
> +                    static int (*resolve_foo(void))(void) { return my_foo; }
> +                    int foo( void ) __attribute__(($1("resolve_foo")));
> +                ],
> +                [leaf], [
> +                    __attribute__(($1)) int foo( void ) { return 0; }
> +                ],
> +                [malloc], [
> +                    void *foo( void ) __attribute__(($1));
> +                ],
> +                [noclone], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [noinline], [
> +                    __attribute__(($1)) int foo( void ) { return 0; }
> +                ],
> +                [nonnull], [
> +                    int foo(char *p) __attribute__(($1(1)));
> +                ],
> +                [noreturn], [
> +                    void foo( void ) __attribute__(($1));
> +                ],
> +                [nothrow], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [optimize], [
> +                    __attribute__(($1(3))) int foo( void ) { return 0; }
> +                ],
> +                [pure], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [sentinel], [
> +                    int foo(void *p, ...) __attribute__(($1));
> +                ],
> +                [sentinel_position], [
> +                    int foo(void *p, ...) __attribute__(($1(1)));
> +                ],
> +                [returns_nonnull], [
> +                    void *foo( void ) __attribute__(($1));
> +                ],
> +                [unused], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [used], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [visibility], [
> +                    int foo_def( void ) __attribute__(($1("default")));
> +                    int foo_hid( void ) __attribute__(($1("hidden")));
> +                    int foo_int( void ) __attribute__(($1("internal")));
> +                    int foo_pro( void ) __attribute__(($1("protected")));
> +                ],
> +                [warning], [
> +                    int foo( void ) __attribute__(($1("")));
> +                ],
> +                [warn_unused_result], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [weak], [
> +                    int foo( void ) __attribute__(($1));
> +                ],
> +                [weakref], [
> +                    static int foo( void ) { return 0; }
> +                    static int bar( void ) __attribute__(($1("foo")));
> +                ],
> +                [
> +                 m4_warn([syntax], [Unsupported attribute $1, the test may fail])
> +                 int foo( void ) __attribute__(($1));
> +                ]
> +            )], [])
> +            ],
> +            dnl GCC doesn't exit with an error if an unknown attribute is
> +            dnl provided but only outputs a warning, so accept the attribute
> +            dnl only if no warning were issued.
> +            [AS_IF([test -s conftest.err],
> +                [AS_VAR_SET([ac_var], [no])],
> +                [AS_VAR_SET([ac_var], [yes])])],
> +            [AS_VAR_SET([ac_var], [no])])
> +    ])
> +
> +    AS_IF([test yes = AS_VAR_GET([ac_var])],
> +        [AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_FUNC_ATTRIBUTE_$1), 1,
> +            [Define to 1 if the system has the `$1' function attribute])], [])
> +
> +    AS_VAR_POPDEF([ac_var])
> +])
> diff --git a/configure.ac b/configure.ac
> index 37096944..639199a9 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -619,6 +619,7 @@ CHECK_CCSUPPORT([-Werror=format-overflow=2], [flg1])
>   CHECK_CCSUPPORT([-Werror=int-conversion], [flg2])
>   CHECK_CCSUPPORT([-Werror=incompatible-pointer-types], [flg3])
>   CHECK_CCSUPPORT([-Werror=misleading-indentation], [flg4])
> +AX_GCC_FUNC_ATTRIBUTE([format])
>   
>   AC_SUBST([AM_CFLAGS], ["$my_am_cflags $flg1 $flg2 $flg3 $flg4"])
>   
> diff --git a/support/include/xcommon.h b/support/include/xcommon.h
> index 23c9a135..30b0403b 100644
> --- a/support/include/xcommon.h
> +++ b/support/include/xcommon.h
> @@ -9,6 +9,10 @@
>   #ifndef _XMALLOC_H
>   #define _MALLOC_H
>   
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
>   #include <sys/types.h>
>   #include <fcntl.h>
>   #include <limits.h>
> @@ -25,9 +29,15 @@
>   
>   #define streq(s, t)	(strcmp ((s), (t)) == 0)
>   
> -/* Functions in sundries.c that are used in mount.c and umount.c  */
> +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
> +#define X_FORMAT(_x) __attribute__((__format__ _x))
> +#else
> +#define X_FORMAT(_x)
> +#endif
> +
> +/* Functions in sundries.c that are used in mount.c and umount.c  */
>   char *canonicalize (const char *path);
> -void nfs_error (const char *fmt, ...);
> +void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
>   void *xmalloc (size_t size);
>   void *xrealloc(void *p, size_t size);
>   void xfree(void *);
> @@ -36,9 +46,9 @@ char *xstrndup (const char *s, int n);
>   char *xstrconcat2 (const char *, const char *);
>   char *xstrconcat3 (const char *, const char *, const char *);
>   char *xstrconcat4 (const char *, const char *, const char *, const char *);
> -void die (int errcode, const char *fmt, ...);
> +void die (int errcode, const char *fmt, ...) X_FORMAT((printf, 2, 3));
>   
> -extern void die(int err, const char *fmt, ...);
> +extern void die(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
>   extern void (*at_die)(void);
>   
>   /* exit status - bits below are ORed */
> diff --git a/support/include/xlog.h b/support/include/xlog.h
> index a11463ed..32ff5a1b 100644
> --- a/support/include/xlog.h
> +++ b/support/include/xlog.h
> @@ -7,6 +7,10 @@
>   #ifndef XLOG_H
>   #define XLOG_H
>   
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
>   #include <stdarg.h>
>   
>   /* These are logged always. L_FATAL also does exit(1) */
> @@ -35,6 +39,12 @@ struct xlog_debugfac {
>   	int		df_fac;
>   };
>   
> +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
> +#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
> +#else
> +#define XLOG_FORMAT(_x)
> +#endif
> +
>   extern int export_errno;
>   void			xlog_open(char *progname);
>   void			xlog_stderr(int on);
> @@ -43,10 +53,10 @@ void			xlog_config(int fac, int on);
>   void			xlog_sconfig(char *, int on);
>   void			xlog_from_conffile(char *);
>   int			xlog_enabled(int fac);
> -void			xlog(int fac, const char *fmt, ...);
> -void			xlog_warn(const char *fmt, ...);
> -void			xlog_err(const char *fmt, ...);
> -void			xlog_errno(int err, const char *fmt, ...);
> -void			xlog_backend(int fac, const char *fmt, va_list args);
> +void			xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
> +void			xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
> +void			xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
> +void			xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
> +void			xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
>   
>   #endif /* XLOG_H */
> diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
> index 4e14430d..976c2d29 100644
> --- a/support/nfs/svc_create.c
> +++ b/support/nfs/svc_create.c
> @@ -184,7 +184,7 @@ svc_create_sock(const struct sockaddr *sap, socklen_t salen,
>   		type = SOCK_STREAM;
>   		break;
>   	default:
> -		xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %u",
> +		xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %lu",
>   			__func__, nconf->nc_semantics);
>   		return -1;
>   	}
> diff --git a/support/nsm/rpc.c b/support/nsm/rpc.c
> index ae49006c..08b4746f 100644
> --- a/support/nsm/rpc.c
> +++ b/support/nsm/rpc.c
> @@ -182,7 +182,7 @@ nsm_xmit_getport(const int sock, const struct sockaddr_in *sin,
>   	uint32_t xid;
>   	XDR xdr;
>   
> -	xlog(D_CALL, "Sending PMAP_GETPORT for %u, %u, udp", program, version);
> +	xlog(D_CALL, "Sending PMAP_GETPORT for %lu, %lu, udp", program, version);
>   
>   	nsm_init_xdrmem(msgbuf, NSM_MAXMSGSIZE, &xdr);
>   	xid = nsm_init_rpc_header(PMAPPROG, PMAPVERS,
> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> index 5cca4175..a04a7898 100644
> --- a/utils/exportfs/exportfs.c
> +++ b/utils/exportfs/exportfs.c
> @@ -651,6 +651,9 @@ out:
>   	return result;
>   }
>   
> +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
> +__attribute__((format (printf, 2, 3)))
> +#endif
>   static char
>   dumpopt(char c, char *fmt, ...)
>   {
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index e25a4337..3861f84a 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -987,8 +987,7 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
>   			} else if (found_type == i && found->m_warned == 0) {
>   				xlog(L_WARNING, "%s exported to both %s and %s, "
>   				     "arbitrarily choosing options from first",
> -				     path, found->m_client->m_hostname, exp->m_client->m_hostname,
> -				     dom);
> +				     path, found->m_client->m_hostname, exp->m_client->m_hostname);
>   				found->m_warned = 1;
>   			}
>   		}
> diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
> index 33571ecb..66366434 100644
> --- a/utils/mountd/mountd.c
> +++ b/utils/mountd/mountd.c
> @@ -210,10 +210,10 @@ killer (int sig)
>   }
>   
>   static void
> -sig_hup (int sig)
> +sig_hup (int UNUSED(sig))
>   {
>   	/* don't exit on SIGHUP */
> -	xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n", sig);
> +	xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n");
>   	return;
>   }
>   
> diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
> index cbf71fc6..7e894e49 100644
> --- a/utils/nfsdcld/nfsdcld.c
> +++ b/utils/nfsdcld/nfsdcld.c
> @@ -212,7 +212,7 @@ cld_inotify_cb(int UNUSED(fd), short which, void *data)
>   	default:
>   		/* anything else is fatal */
>   		xlog(L_FATAL, "%s: unable to open new pipe (%d). Aborting.",
> -			ret, __func__);
> +			__func__, ret);
>   		exit(ret);
>   	}
>   
> diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
> index fa81df87..afa63499 100644
> --- a/utils/nfsdcld/sqlite.c
> +++ b/utils/nfsdcld/sqlite.c
> @@ -473,7 +473,7 @@ sqlite_fix_table_name(const char *name)
>   	}
>   	ret = sqlite3_exec(dbh, (const char *)buf, NULL, NULL, &err);
>   	if (ret != SQLITE_OK) {
> -		xlog(L_ERROR, "Unable to fix table for epoch %d: %s",
> +		xlog(L_ERROR, "Unable to fix table for epoch %"PRIu64": %s",
>   		     val, err);
>   		goto out;
>   	}
> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
> index fc00da7a..cf7f65e9 100644
> --- a/utils/nfsidmap/nfsidmap.c
> +++ b/utils/nfsidmap/nfsidmap.c
> @@ -18,7 +18,7 @@
>   #include "xcommon.h"
>   
>   int verbose = 0;
> -char *usage = "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]";
> +#define USAGE "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]"
>   
>   #define MAX_ID_LEN   11
>   #define IDMAP_NAMESZ 128
> @@ -403,7 +403,7 @@ int main(int argc, char **argv)
>   			break;
>   		case 'h':
>   		default:
> -			xlog_warn(usage, progname);
> +			xlog_warn(USAGE, progname);
>   			exit(opt == 'h' ? 0 : 1);
>   		}
>   	}
> @@ -435,7 +435,7 @@ int main(int argc, char **argv)
>   	xlog_stderr(verbose);
>   	if ((argc - optind) != 2) {
>   		xlog_warn("Bad arg count. Check /etc/request-key.conf");
> -		xlog_warn(usage, progname);
> +		xlog_warn(USAGE, progname);
>   		return EXIT_FAILURE;
>   	}
>   
> @@ -453,7 +453,7 @@ int main(int argc, char **argv)
>   		return EXIT_FAILURE;
>   	}
>   	if (verbose) {
> -		xlog_warn("key: 0x%lx type: %s value: %s timeout %ld",
> +		xlog_warn("key: 0x%x type: %s value: %s timeout %d",
>   			key, type, value, timeout);
>   	}
>   
> diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
> index c4f6364f..5b261480 100644
> --- a/utils/statd/rmtcall.c
> +++ b/utils/statd/rmtcall.c
> @@ -247,7 +247,7 @@ process_reply(FD_SET_TYPE *rfds)
>   		xlog_warn("%s: service %d not registered on localhost",
>   			__func__, NL_MY_PROG(lp));
>   	} else {
> -		xlog(D_GENERAL, "%s: Callback to %s (for %d) succeeded",
> +		xlog(D_GENERAL, "%s: Callback to %s (for %s) succeeded",
>   			__func__, NL_MY_NAME(lp), NL_MON_NAME(lp));
>   	}
>   	nlist_free(&notify, lp);
> diff --git a/utils/statd/statd.c b/utils/statd/statd.c
> index 14673800..8eef2ff2 100644
> --- a/utils/statd/statd.c
> +++ b/utils/statd/statd.c
> @@ -136,7 +136,7 @@ static void log_modes(void)
>   	strcat(buf, "TI-RPC ");
>   #endif
>   
> -	xlog_warn(buf);
> +	xlog_warn("%s", buf);
>   }
>   
>   /*
> diff --git a/utils/statd/svc_run.c b/utils/statd/svc_run.c
> index d1dbd74a..e343c768 100644
> --- a/utils/statd/svc_run.c
> +++ b/utils/statd/svc_run.c
> @@ -53,6 +53,7 @@
>   
>   #include <errno.h>
>   #include <time.h>
> +#include <inttypes.h>
>   #include "statd.h"
>   #include "notlist.h"
>   
> @@ -104,8 +105,8 @@ my_svc_run(int sockfd)
>   
>   			tv.tv_sec  = NL_WHEN(notify) - now;
>   			tv.tv_usec = 0;
> -			xlog(D_GENERAL, "Waiting for reply... (timeo %d)",
> -							tv.tv_sec);
> +			xlog(D_GENERAL, "Waiting for reply... (timeo %jd)",
> +							(intmax_t)tv.tv_sec);
>   			selret = select(FD_SETSIZE, &readfds,
>   				(void *) 0, (void *) 0, &tv);
>   		} else {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [nfs-utils PATCH v2] Add printf format checking
  2019-09-16 18:33   ` Joshua Watt
@ 2019-09-20 14:05     ` Steve Dickson
  2019-09-23 14:04       ` Joshua Watt
  0 siblings, 1 reply; 5+ messages in thread
From: Steve Dickson @ 2019-09-20 14:05 UTC (permalink / raw)
  To: Joshua Watt, linux-nfs



On 9/16/19 2:33 PM, Joshua Watt wrote:
> On 9/6/19 3:24 PM, Joshua Watt wrote:
>> Adds a configure time check for __attribute__((format)) and then uses it
>> to enforce checking the format of several printf-like functions. Several
>> invalid uses of format codes that were discovered have now been fixed.
>>
>> V2: Fix use of "%jd" format code on an argument that wasn't intmax_t
> 
> ping?
> 
>>
>> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
Committed... Thanks for the ping!

steved.
>> ---
>>   aclocal/ax_gcc_func_attribute.m4 | 238 +++++++++++++++++++++++++++++++
>>   configure.ac                     |   1 +
>>   support/include/xcommon.h        |  18 ++-
>>   support/include/xlog.h           |  20 ++-
>>   support/nfs/svc_create.c         |   2 +-
>>   support/nsm/rpc.c                |   2 +-
>>   utils/exportfs/exportfs.c        |   3 +
>>   utils/mountd/cache.c             |   3 +-
>>   utils/mountd/mountd.c            |   4 +-
>>   utils/nfsdcld/nfsdcld.c          |   2 +-
>>   utils/nfsdcld/sqlite.c           |   2 +-
>>   utils/nfsidmap/nfsidmap.c        |   8 +-
>>   utils/statd/rmtcall.c            |   2 +-
>>   utils/statd/statd.c              |   2 +-
>>   utils/statd/svc_run.c            |   5 +-
>>   15 files changed, 287 insertions(+), 25 deletions(-)
>>   create mode 100644 aclocal/ax_gcc_func_attribute.m4
>>
>> diff --git a/aclocal/ax_gcc_func_attribute.m4 b/aclocal/ax_gcc_func_attribute.m4
>> new file mode 100644
>> index 00000000..098c9aad
>> --- /dev/null
>> +++ b/aclocal/ax_gcc_func_attribute.m4
>> @@ -0,0 +1,238 @@
>> +# ===========================================================================
>> +#  https://www.gnu.org/software/autoconf-archive/ax_gcc_func_attribute.html
>> +# ===========================================================================
>> +#
>> +# SYNOPSIS
>> +#
>> +#   AX_GCC_FUNC_ATTRIBUTE(ATTRIBUTE)
>> +#
>> +# DESCRIPTION
>> +#
>> +#   This macro checks if the compiler supports one of GCC's function
>> +#   attributes; many other compilers also provide function attributes with
>> +#   the same syntax. Compiler warnings are used to detect supported
>> +#   attributes as unsupported ones are ignored by default so quieting
>> +#   warnings when using this macro will yield false positives.
>> +#
>> +#   The ATTRIBUTE parameter holds the name of the attribute to be checked.
>> +#
>> +#   If ATTRIBUTE is supported define HAVE_FUNC_ATTRIBUTE_<ATTRIBUTE>.
>> +#
>> +#   The macro caches its result in the ax_cv_have_func_attribute_<attribute>
>> +#   variable.
>> +#
>> +#   The macro currently supports the following function attributes:
>> +#
>> +#    alias
>> +#    aligned
>> +#    alloc_size
>> +#    always_inline
>> +#    artificial
>> +#    cold
>> +#    const
>> +#    constructor
>> +#    constructor_priority for constructor attribute with priority
>> +#    deprecated
>> +#    destructor
>> +#    dllexport
>> +#    dllimport
>> +#    error
>> +#    externally_visible
>> +#    fallthrough
>> +#    flatten
>> +#    format
>> +#    format_arg
>> +#    gnu_inline
>> +#    hot
>> +#    ifunc
>> +#    leaf
>> +#    malloc
>> +#    noclone
>> +#    noinline
>> +#    nonnull
>> +#    noreturn
>> +#    nothrow
>> +#    optimize
>> +#    pure
>> +#    sentinel
>> +#    sentinel_position
>> +#    unused
>> +#    used
>> +#    visibility
>> +#    warning
>> +#    warn_unused_result
>> +#    weak
>> +#    weakref
>> +#
>> +#   Unsupported function attributes will be tested with a prototype
>> +#   returning an int and not accepting any arguments and the result of the
>> +#   check might be wrong or meaningless so use with care.
>> +#
>> +# LICENSE
>> +#
>> +#   Copyright (c) 2013 Gabriele Svelto <gabriele.svelto@gmail.com>
>> +#
>> +#   Copying and distribution of this file, with or without modification, are
>> +#   permitted in any medium without royalty provided the copyright notice
>> +#   and this notice are preserved.  This file is offered as-is, without any
>> +#   warranty.
>> +
>> +#serial 9
>> +
>> +AC_DEFUN([AX_GCC_FUNC_ATTRIBUTE], [
>> +    AS_VAR_PUSHDEF([ac_var], [ax_cv_have_func_attribute_$1])
>> +
>> +    AC_CACHE_CHECK([for __attribute__(($1))], [ac_var], [
>> +        AC_LINK_IFELSE([AC_LANG_PROGRAM([
>> +            m4_case([$1],
>> +                [alias], [
>> +                    int foo( void ) { return 0; }
>> +                    int bar( void ) __attribute__(($1("foo")));
>> +                ],
>> +                [aligned], [
>> +                    int foo( void ) __attribute__(($1(32)));
>> +                ],
>> +                [alloc_size], [
>> +                    void *foo(int a) __attribute__(($1(1)));
>> +                ],
>> +                [always_inline], [
>> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
>> +                ],
>> +                [artificial], [
>> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
>> +                ],
>> +                [cold], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [const], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [constructor_priority], [
>> +                    int foo( void ) __attribute__((__constructor__(65535/2)));
>> +                ],
>> +                [constructor], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [deprecated], [
>> +                    int foo( void ) __attribute__(($1("")));
>> +                ],
>> +                [destructor], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [dllexport], [
>> +                    __attribute__(($1)) int foo( void ) { return 0; }
>> +                ],
>> +                [dllimport], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [error], [
>> +                    int foo( void ) __attribute__(($1("")));
>> +                ],
>> +                [externally_visible], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [fallthrough], [
>> +                    int foo( void ) {switch (0) { case 1: __attribute__(($1)); case 2: break ; }};
>> +                ],
>> +                [flatten], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [format], [
>> +                    int foo(const char *p, ...) __attribute__(($1(printf, 1, 2)));
>> +                ],
>> +                [format_arg], [
>> +                    char *foo(const char *p) __attribute__(($1(1)));
>> +                ],
>> +                [gnu_inline], [
>> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
>> +                ],
>> +                [hot], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [ifunc], [
>> +                    int my_foo( void ) { return 0; }
>> +                    static int (*resolve_foo(void))(void) { return my_foo; }
>> +                    int foo( void ) __attribute__(($1("resolve_foo")));
>> +                ],
>> +                [leaf], [
>> +                    __attribute__(($1)) int foo( void ) { return 0; }
>> +                ],
>> +                [malloc], [
>> +                    void *foo( void ) __attribute__(($1));
>> +                ],
>> +                [noclone], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [noinline], [
>> +                    __attribute__(($1)) int foo( void ) { return 0; }
>> +                ],
>> +                [nonnull], [
>> +                    int foo(char *p) __attribute__(($1(1)));
>> +                ],
>> +                [noreturn], [
>> +                    void foo( void ) __attribute__(($1));
>> +                ],
>> +                [nothrow], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [optimize], [
>> +                    __attribute__(($1(3))) int foo( void ) { return 0; }
>> +                ],
>> +                [pure], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [sentinel], [
>> +                    int foo(void *p, ...) __attribute__(($1));
>> +                ],
>> +                [sentinel_position], [
>> +                    int foo(void *p, ...) __attribute__(($1(1)));
>> +                ],
>> +                [returns_nonnull], [
>> +                    void *foo( void ) __attribute__(($1));
>> +                ],
>> +                [unused], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [used], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [visibility], [
>> +                    int foo_def( void ) __attribute__(($1("default")));
>> +                    int foo_hid( void ) __attribute__(($1("hidden")));
>> +                    int foo_int( void ) __attribute__(($1("internal")));
>> +                    int foo_pro( void ) __attribute__(($1("protected")));
>> +                ],
>> +                [warning], [
>> +                    int foo( void ) __attribute__(($1("")));
>> +                ],
>> +                [warn_unused_result], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [weak], [
>> +                    int foo( void ) __attribute__(($1));
>> +                ],
>> +                [weakref], [
>> +                    static int foo( void ) { return 0; }
>> +                    static int bar( void ) __attribute__(($1("foo")));
>> +                ],
>> +                [
>> +                 m4_warn([syntax], [Unsupported attribute $1, the test may fail])
>> +                 int foo( void ) __attribute__(($1));
>> +                ]
>> +            )], [])
>> +            ],
>> +            dnl GCC doesn't exit with an error if an unknown attribute is
>> +            dnl provided but only outputs a warning, so accept the attribute
>> +            dnl only if no warning were issued.
>> +            [AS_IF([test -s conftest.err],
>> +                [AS_VAR_SET([ac_var], [no])],
>> +                [AS_VAR_SET([ac_var], [yes])])],
>> +            [AS_VAR_SET([ac_var], [no])])
>> +    ])
>> +
>> +    AS_IF([test yes = AS_VAR_GET([ac_var])],
>> +        [AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_FUNC_ATTRIBUTE_$1), 1,
>> +            [Define to 1 if the system has the `$1' function attribute])], [])
>> +
>> +    AS_VAR_POPDEF([ac_var])
>> +])
>> diff --git a/configure.ac b/configure.ac
>> index 37096944..639199a9 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -619,6 +619,7 @@ CHECK_CCSUPPORT([-Werror=format-overflow=2], [flg1])
>>   CHECK_CCSUPPORT([-Werror=int-conversion], [flg2])
>>   CHECK_CCSUPPORT([-Werror=incompatible-pointer-types], [flg3])
>>   CHECK_CCSUPPORT([-Werror=misleading-indentation], [flg4])
>> +AX_GCC_FUNC_ATTRIBUTE([format])
>>     AC_SUBST([AM_CFLAGS], ["$my_am_cflags $flg1 $flg2 $flg3 $flg4"])
>>   diff --git a/support/include/xcommon.h b/support/include/xcommon.h
>> index 23c9a135..30b0403b 100644
>> --- a/support/include/xcommon.h
>> +++ b/support/include/xcommon.h
>> @@ -9,6 +9,10 @@
>>   #ifndef _XMALLOC_H
>>   #define _MALLOC_H
>>   +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +
>>   #include <sys/types.h>
>>   #include <fcntl.h>
>>   #include <limits.h>
>> @@ -25,9 +29,15 @@
>>     #define streq(s, t)    (strcmp ((s), (t)) == 0)
>>   -/* Functions in sundries.c that are used in mount.c and umount.c  */
>> +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
>> +#define X_FORMAT(_x) __attribute__((__format__ _x))
>> +#else
>> +#define X_FORMAT(_x)
>> +#endif
>> +
>> +/* Functions in sundries.c that are used in mount.c and umount.c  */
>>   char *canonicalize (const char *path);
>> -void nfs_error (const char *fmt, ...);
>> +void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
>>   void *xmalloc (size_t size);
>>   void *xrealloc(void *p, size_t size);
>>   void xfree(void *);
>> @@ -36,9 +46,9 @@ char *xstrndup (const char *s, int n);
>>   char *xstrconcat2 (const char *, const char *);
>>   char *xstrconcat3 (const char *, const char *, const char *);
>>   char *xstrconcat4 (const char *, const char *, const char *, const char *);
>> -void die (int errcode, const char *fmt, ...);
>> +void die (int errcode, const char *fmt, ...) X_FORMAT((printf, 2, 3));
>>   -extern void die(int err, const char *fmt, ...);
>> +extern void die(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
>>   extern void (*at_die)(void);
>>     /* exit status - bits below are ORed */
>> diff --git a/support/include/xlog.h b/support/include/xlog.h
>> index a11463ed..32ff5a1b 100644
>> --- a/support/include/xlog.h
>> +++ b/support/include/xlog.h
>> @@ -7,6 +7,10 @@
>>   #ifndef XLOG_H
>>   #define XLOG_H
>>   +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +
>>   #include <stdarg.h>
>>     /* These are logged always. L_FATAL also does exit(1) */
>> @@ -35,6 +39,12 @@ struct xlog_debugfac {
>>       int        df_fac;
>>   };
>>   +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
>> +#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
>> +#else
>> +#define XLOG_FORMAT(_x)
>> +#endif
>> +
>>   extern int export_errno;
>>   void            xlog_open(char *progname);
>>   void            xlog_stderr(int on);
>> @@ -43,10 +53,10 @@ void            xlog_config(int fac, int on);
>>   void            xlog_sconfig(char *, int on);
>>   void            xlog_from_conffile(char *);
>>   int            xlog_enabled(int fac);
>> -void            xlog(int fac, const char *fmt, ...);
>> -void            xlog_warn(const char *fmt, ...);
>> -void            xlog_err(const char *fmt, ...);
>> -void            xlog_errno(int err, const char *fmt, ...);
>> -void            xlog_backend(int fac, const char *fmt, va_list args);
>> +void            xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
>> +void            xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
>> +void            xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
>> +void            xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
>> +void            xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
>>     #endif /* XLOG_H */
>> diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
>> index 4e14430d..976c2d29 100644
>> --- a/support/nfs/svc_create.c
>> +++ b/support/nfs/svc_create.c
>> @@ -184,7 +184,7 @@ svc_create_sock(const struct sockaddr *sap, socklen_t salen,
>>           type = SOCK_STREAM;
>>           break;
>>       default:
>> -        xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %u",
>> +        xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %lu",
>>               __func__, nconf->nc_semantics);
>>           return -1;
>>       }
>> diff --git a/support/nsm/rpc.c b/support/nsm/rpc.c
>> index ae49006c..08b4746f 100644
>> --- a/support/nsm/rpc.c
>> +++ b/support/nsm/rpc.c
>> @@ -182,7 +182,7 @@ nsm_xmit_getport(const int sock, const struct sockaddr_in *sin,
>>       uint32_t xid;
>>       XDR xdr;
>>   -    xlog(D_CALL, "Sending PMAP_GETPORT for %u, %u, udp", program, version);
>> +    xlog(D_CALL, "Sending PMAP_GETPORT for %lu, %lu, udp", program, version);
>>         nsm_init_xdrmem(msgbuf, NSM_MAXMSGSIZE, &xdr);
>>       xid = nsm_init_rpc_header(PMAPPROG, PMAPVERS,
>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>> index 5cca4175..a04a7898 100644
>> --- a/utils/exportfs/exportfs.c
>> +++ b/utils/exportfs/exportfs.c
>> @@ -651,6 +651,9 @@ out:
>>       return result;
>>   }
>>   +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
>> +__attribute__((format (printf, 2, 3)))
>> +#endif
>>   static char
>>   dumpopt(char c, char *fmt, ...)
>>   {
>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>> index e25a4337..3861f84a 100644
>> --- a/utils/mountd/cache.c
>> +++ b/utils/mountd/cache.c
>> @@ -987,8 +987,7 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
>>               } else if (found_type == i && found->m_warned == 0) {
>>                   xlog(L_WARNING, "%s exported to both %s and %s, "
>>                        "arbitrarily choosing options from first",
>> -                     path, found->m_client->m_hostname, exp->m_client->m_hostname,
>> -                     dom);
>> +                     path, found->m_client->m_hostname, exp->m_client->m_hostname);
>>                   found->m_warned = 1;
>>               }
>>           }
>> diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
>> index 33571ecb..66366434 100644
>> --- a/utils/mountd/mountd.c
>> +++ b/utils/mountd/mountd.c
>> @@ -210,10 +210,10 @@ killer (int sig)
>>   }
>>     static void
>> -sig_hup (int sig)
>> +sig_hup (int UNUSED(sig))
>>   {
>>       /* don't exit on SIGHUP */
>> -    xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n", sig);
>> +    xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n");
>>       return;
>>   }
>>   diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
>> index cbf71fc6..7e894e49 100644
>> --- a/utils/nfsdcld/nfsdcld.c
>> +++ b/utils/nfsdcld/nfsdcld.c
>> @@ -212,7 +212,7 @@ cld_inotify_cb(int UNUSED(fd), short which, void *data)
>>       default:
>>           /* anything else is fatal */
>>           xlog(L_FATAL, "%s: unable to open new pipe (%d). Aborting.",
>> -            ret, __func__);
>> +            __func__, ret);
>>           exit(ret);
>>       }
>>   diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
>> index fa81df87..afa63499 100644
>> --- a/utils/nfsdcld/sqlite.c
>> +++ b/utils/nfsdcld/sqlite.c
>> @@ -473,7 +473,7 @@ sqlite_fix_table_name(const char *name)
>>       }
>>       ret = sqlite3_exec(dbh, (const char *)buf, NULL, NULL, &err);
>>       if (ret != SQLITE_OK) {
>> -        xlog(L_ERROR, "Unable to fix table for epoch %d: %s",
>> +        xlog(L_ERROR, "Unable to fix table for epoch %"PRIu64": %s",
>>                val, err);
>>           goto out;
>>       }
>> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
>> index fc00da7a..cf7f65e9 100644
>> --- a/utils/nfsidmap/nfsidmap.c
>> +++ b/utils/nfsidmap/nfsidmap.c
>> @@ -18,7 +18,7 @@
>>   #include "xcommon.h"
>>     int verbose = 0;
>> -char *usage = "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]";
>> +#define USAGE "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]"
>>     #define MAX_ID_LEN   11
>>   #define IDMAP_NAMESZ 128
>> @@ -403,7 +403,7 @@ int main(int argc, char **argv)
>>               break;
>>           case 'h':
>>           default:
>> -            xlog_warn(usage, progname);
>> +            xlog_warn(USAGE, progname);
>>               exit(opt == 'h' ? 0 : 1);
>>           }
>>       }
>> @@ -435,7 +435,7 @@ int main(int argc, char **argv)
>>       xlog_stderr(verbose);
>>       if ((argc - optind) != 2) {
>>           xlog_warn("Bad arg count. Check /etc/request-key.conf");
>> -        xlog_warn(usage, progname);
>> +        xlog_warn(USAGE, progname);
>>           return EXIT_FAILURE;
>>       }
>>   @@ -453,7 +453,7 @@ int main(int argc, char **argv)
>>           return EXIT_FAILURE;
>>       }
>>       if (verbose) {
>> -        xlog_warn("key: 0x%lx type: %s value: %s timeout %ld",
>> +        xlog_warn("key: 0x%x type: %s value: %s timeout %d",
>>               key, type, value, timeout);
>>       }
>>   diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
>> index c4f6364f..5b261480 100644
>> --- a/utils/statd/rmtcall.c
>> +++ b/utils/statd/rmtcall.c
>> @@ -247,7 +247,7 @@ process_reply(FD_SET_TYPE *rfds)
>>           xlog_warn("%s: service %d not registered on localhost",
>>               __func__, NL_MY_PROG(lp));
>>       } else {
>> -        xlog(D_GENERAL, "%s: Callback to %s (for %d) succeeded",
>> +        xlog(D_GENERAL, "%s: Callback to %s (for %s) succeeded",
>>               __func__, NL_MY_NAME(lp), NL_MON_NAME(lp));
>>       }
>>       nlist_free(&notify, lp);
>> diff --git a/utils/statd/statd.c b/utils/statd/statd.c
>> index 14673800..8eef2ff2 100644
>> --- a/utils/statd/statd.c
>> +++ b/utils/statd/statd.c
>> @@ -136,7 +136,7 @@ static void log_modes(void)
>>       strcat(buf, "TI-RPC ");
>>   #endif
>>   -    xlog_warn(buf);
>> +    xlog_warn("%s", buf);
>>   }
>>     /*
>> diff --git a/utils/statd/svc_run.c b/utils/statd/svc_run.c
>> index d1dbd74a..e343c768 100644
>> --- a/utils/statd/svc_run.c
>> +++ b/utils/statd/svc_run.c
>> @@ -53,6 +53,7 @@
>>     #include <errno.h>
>>   #include <time.h>
>> +#include <inttypes.h>
>>   #include "statd.h"
>>   #include "notlist.h"
>>   @@ -104,8 +105,8 @@ my_svc_run(int sockfd)
>>                 tv.tv_sec  = NL_WHEN(notify) - now;
>>               tv.tv_usec = 0;
>> -            xlog(D_GENERAL, "Waiting for reply... (timeo %d)",
>> -                            tv.tv_sec);
>> +            xlog(D_GENERAL, "Waiting for reply... (timeo %jd)",
>> +                            (intmax_t)tv.tv_sec);
>>               selret = select(FD_SETSIZE, &readfds,
>>                   (void *) 0, (void *) 0, &tv);
>>           } else {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [nfs-utils PATCH v2] Add printf format checking
  2019-09-20 14:05     ` Steve Dickson
@ 2019-09-23 14:04       ` Joshua Watt
  0 siblings, 0 replies; 5+ messages in thread
From: Joshua Watt @ 2019-09-23 14:04 UTC (permalink / raw)
  To: Steve Dickson, linux-nfs


On 9/20/19 9:05 AM, Steve Dickson wrote:
>
> On 9/16/19 2:33 PM, Joshua Watt wrote:
>> On 9/6/19 3:24 PM, Joshua Watt wrote:
>>> Adds a configure time check for __attribute__((format)) and then uses it
>>> to enforce checking the format of several printf-like functions. Several
>>> invalid uses of format codes that were discovered have now been fixed.
>>>
>>> V2: Fix use of "%jd" format code on an argument that wasn't intmax_t
>> ping?
>>
>>> Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
> Committed... Thanks for the ping!

I haven't seen this commit land in the git repository 
git.linux-nfs.org/projects/steved/nfs-utils.git yet. Is there a 
different one I should be looking at?

Thanks

>
> steved.
>>> ---
>>>    aclocal/ax_gcc_func_attribute.m4 | 238 +++++++++++++++++++++++++++++++
>>>    configure.ac                     |   1 +
>>>    support/include/xcommon.h        |  18 ++-
>>>    support/include/xlog.h           |  20 ++-
>>>    support/nfs/svc_create.c         |   2 +-
>>>    support/nsm/rpc.c                |   2 +-
>>>    utils/exportfs/exportfs.c        |   3 +
>>>    utils/mountd/cache.c             |   3 +-
>>>    utils/mountd/mountd.c            |   4 +-
>>>    utils/nfsdcld/nfsdcld.c          |   2 +-
>>>    utils/nfsdcld/sqlite.c           |   2 +-
>>>    utils/nfsidmap/nfsidmap.c        |   8 +-
>>>    utils/statd/rmtcall.c            |   2 +-
>>>    utils/statd/statd.c              |   2 +-
>>>    utils/statd/svc_run.c            |   5 +-
>>>    15 files changed, 287 insertions(+), 25 deletions(-)
>>>    create mode 100644 aclocal/ax_gcc_func_attribute.m4
>>>
>>> diff --git a/aclocal/ax_gcc_func_attribute.m4 b/aclocal/ax_gcc_func_attribute.m4
>>> new file mode 100644
>>> index 00000000..098c9aad
>>> --- /dev/null
>>> +++ b/aclocal/ax_gcc_func_attribute.m4
>>> @@ -0,0 +1,238 @@
>>> +# ===========================================================================
>>> +#  https://www.gnu.org/software/autoconf-archive/ax_gcc_func_attribute.html
>>> +# ===========================================================================
>>> +#
>>> +# SYNOPSIS
>>> +#
>>> +#   AX_GCC_FUNC_ATTRIBUTE(ATTRIBUTE)
>>> +#
>>> +# DESCRIPTION
>>> +#
>>> +#   This macro checks if the compiler supports one of GCC's function
>>> +#   attributes; many other compilers also provide function attributes with
>>> +#   the same syntax. Compiler warnings are used to detect supported
>>> +#   attributes as unsupported ones are ignored by default so quieting
>>> +#   warnings when using this macro will yield false positives.
>>> +#
>>> +#   The ATTRIBUTE parameter holds the name of the attribute to be checked.
>>> +#
>>> +#   If ATTRIBUTE is supported define HAVE_FUNC_ATTRIBUTE_<ATTRIBUTE>.
>>> +#
>>> +#   The macro caches its result in the ax_cv_have_func_attribute_<attribute>
>>> +#   variable.
>>> +#
>>> +#   The macro currently supports the following function attributes:
>>> +#
>>> +#    alias
>>> +#    aligned
>>> +#    alloc_size
>>> +#    always_inline
>>> +#    artificial
>>> +#    cold
>>> +#    const
>>> +#    constructor
>>> +#    constructor_priority for constructor attribute with priority
>>> +#    deprecated
>>> +#    destructor
>>> +#    dllexport
>>> +#    dllimport
>>> +#    error
>>> +#    externally_visible
>>> +#    fallthrough
>>> +#    flatten
>>> +#    format
>>> +#    format_arg
>>> +#    gnu_inline
>>> +#    hot
>>> +#    ifunc
>>> +#    leaf
>>> +#    malloc
>>> +#    noclone
>>> +#    noinline
>>> +#    nonnull
>>> +#    noreturn
>>> +#    nothrow
>>> +#    optimize
>>> +#    pure
>>> +#    sentinel
>>> +#    sentinel_position
>>> +#    unused
>>> +#    used
>>> +#    visibility
>>> +#    warning
>>> +#    warn_unused_result
>>> +#    weak
>>> +#    weakref
>>> +#
>>> +#   Unsupported function attributes will be tested with a prototype
>>> +#   returning an int and not accepting any arguments and the result of the
>>> +#   check might be wrong or meaningless so use with care.
>>> +#
>>> +# LICENSE
>>> +#
>>> +#   Copyright (c) 2013 Gabriele Svelto <gabriele.svelto@gmail.com>
>>> +#
>>> +#   Copying and distribution of this file, with or without modification, are
>>> +#   permitted in any medium without royalty provided the copyright notice
>>> +#   and this notice are preserved.  This file is offered as-is, without any
>>> +#   warranty.
>>> +
>>> +#serial 9
>>> +
>>> +AC_DEFUN([AX_GCC_FUNC_ATTRIBUTE], [
>>> +    AS_VAR_PUSHDEF([ac_var], [ax_cv_have_func_attribute_$1])
>>> +
>>> +    AC_CACHE_CHECK([for __attribute__(($1))], [ac_var], [
>>> +        AC_LINK_IFELSE([AC_LANG_PROGRAM([
>>> +            m4_case([$1],
>>> +                [alias], [
>>> +                    int foo( void ) { return 0; }
>>> +                    int bar( void ) __attribute__(($1("foo")));
>>> +                ],
>>> +                [aligned], [
>>> +                    int foo( void ) __attribute__(($1(32)));
>>> +                ],
>>> +                [alloc_size], [
>>> +                    void *foo(int a) __attribute__(($1(1)));
>>> +                ],
>>> +                [always_inline], [
>>> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
>>> +                ],
>>> +                [artificial], [
>>> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
>>> +                ],
>>> +                [cold], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [const], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [constructor_priority], [
>>> +                    int foo( void ) __attribute__((__constructor__(65535/2)));
>>> +                ],
>>> +                [constructor], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [deprecated], [
>>> +                    int foo( void ) __attribute__(($1("")));
>>> +                ],
>>> +                [destructor], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [dllexport], [
>>> +                    __attribute__(($1)) int foo( void ) { return 0; }
>>> +                ],
>>> +                [dllimport], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [error], [
>>> +                    int foo( void ) __attribute__(($1("")));
>>> +                ],
>>> +                [externally_visible], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [fallthrough], [
>>> +                    int foo( void ) {switch (0) { case 1: __attribute__(($1)); case 2: break ; }};
>>> +                ],
>>> +                [flatten], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [format], [
>>> +                    int foo(const char *p, ...) __attribute__(($1(printf, 1, 2)));
>>> +                ],
>>> +                [format_arg], [
>>> +                    char *foo(const char *p) __attribute__(($1(1)));
>>> +                ],
>>> +                [gnu_inline], [
>>> +                    inline __attribute__(($1)) int foo( void ) { return 0; }
>>> +                ],
>>> +                [hot], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [ifunc], [
>>> +                    int my_foo( void ) { return 0; }
>>> +                    static int (*resolve_foo(void))(void) { return my_foo; }
>>> +                    int foo( void ) __attribute__(($1("resolve_foo")));
>>> +                ],
>>> +                [leaf], [
>>> +                    __attribute__(($1)) int foo( void ) { return 0; }
>>> +                ],
>>> +                [malloc], [
>>> +                    void *foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [noclone], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [noinline], [
>>> +                    __attribute__(($1)) int foo( void ) { return 0; }
>>> +                ],
>>> +                [nonnull], [
>>> +                    int foo(char *p) __attribute__(($1(1)));
>>> +                ],
>>> +                [noreturn], [
>>> +                    void foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [nothrow], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [optimize], [
>>> +                    __attribute__(($1(3))) int foo( void ) { return 0; }
>>> +                ],
>>> +                [pure], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [sentinel], [
>>> +                    int foo(void *p, ...) __attribute__(($1));
>>> +                ],
>>> +                [sentinel_position], [
>>> +                    int foo(void *p, ...) __attribute__(($1(1)));
>>> +                ],
>>> +                [returns_nonnull], [
>>> +                    void *foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [unused], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [used], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [visibility], [
>>> +                    int foo_def( void ) __attribute__(($1("default")));
>>> +                    int foo_hid( void ) __attribute__(($1("hidden")));
>>> +                    int foo_int( void ) __attribute__(($1("internal")));
>>> +                    int foo_pro( void ) __attribute__(($1("protected")));
>>> +                ],
>>> +                [warning], [
>>> +                    int foo( void ) __attribute__(($1("")));
>>> +                ],
>>> +                [warn_unused_result], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [weak], [
>>> +                    int foo( void ) __attribute__(($1));
>>> +                ],
>>> +                [weakref], [
>>> +                    static int foo( void ) { return 0; }
>>> +                    static int bar( void ) __attribute__(($1("foo")));
>>> +                ],
>>> +                [
>>> +                 m4_warn([syntax], [Unsupported attribute $1, the test may fail])
>>> +                 int foo( void ) __attribute__(($1));
>>> +                ]
>>> +            )], [])
>>> +            ],
>>> +            dnl GCC doesn't exit with an error if an unknown attribute is
>>> +            dnl provided but only outputs a warning, so accept the attribute
>>> +            dnl only if no warning were issued.
>>> +            [AS_IF([test -s conftest.err],
>>> +                [AS_VAR_SET([ac_var], [no])],
>>> +                [AS_VAR_SET([ac_var], [yes])])],
>>> +            [AS_VAR_SET([ac_var], [no])])
>>> +    ])
>>> +
>>> +    AS_IF([test yes = AS_VAR_GET([ac_var])],
>>> +        [AC_DEFINE_UNQUOTED(AS_TR_CPP(HAVE_FUNC_ATTRIBUTE_$1), 1,
>>> +            [Define to 1 if the system has the `$1' function attribute])], [])
>>> +
>>> +    AS_VAR_POPDEF([ac_var])
>>> +])
>>> diff --git a/configure.ac b/configure.ac
>>> index 37096944..639199a9 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -619,6 +619,7 @@ CHECK_CCSUPPORT([-Werror=format-overflow=2], [flg1])
>>>    CHECK_CCSUPPORT([-Werror=int-conversion], [flg2])
>>>    CHECK_CCSUPPORT([-Werror=incompatible-pointer-types], [flg3])
>>>    CHECK_CCSUPPORT([-Werror=misleading-indentation], [flg4])
>>> +AX_GCC_FUNC_ATTRIBUTE([format])
>>>      AC_SUBST([AM_CFLAGS], ["$my_am_cflags $flg1 $flg2 $flg3 $flg4"])
>>>    diff --git a/support/include/xcommon.h b/support/include/xcommon.h
>>> index 23c9a135..30b0403b 100644
>>> --- a/support/include/xcommon.h
>>> +++ b/support/include/xcommon.h
>>> @@ -9,6 +9,10 @@
>>>    #ifndef _XMALLOC_H
>>>    #define _MALLOC_H
>>>    +#ifdef HAVE_CONFIG_H
>>> +#include <config.h>
>>> +#endif
>>> +
>>>    #include <sys/types.h>
>>>    #include <fcntl.h>
>>>    #include <limits.h>
>>> @@ -25,9 +29,15 @@
>>>      #define streq(s, t)    (strcmp ((s), (t)) == 0)
>>>    -/* Functions in sundries.c that are used in mount.c and umount.c  */
>>> +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
>>> +#define X_FORMAT(_x) __attribute__((__format__ _x))
>>> +#else
>>> +#define X_FORMAT(_x)
>>> +#endif
>>> +
>>> +/* Functions in sundries.c that are used in mount.c and umount.c  */
>>>    char *canonicalize (const char *path);
>>> -void nfs_error (const char *fmt, ...);
>>> +void nfs_error (const char *fmt, ...) X_FORMAT((printf, 1, 2));
>>>    void *xmalloc (size_t size);
>>>    void *xrealloc(void *p, size_t size);
>>>    void xfree(void *);
>>> @@ -36,9 +46,9 @@ char *xstrndup (const char *s, int n);
>>>    char *xstrconcat2 (const char *, const char *);
>>>    char *xstrconcat3 (const char *, const char *, const char *);
>>>    char *xstrconcat4 (const char *, const char *, const char *, const char *);
>>> -void die (int errcode, const char *fmt, ...);
>>> +void die (int errcode, const char *fmt, ...) X_FORMAT((printf, 2, 3));
>>>    -extern void die(int err, const char *fmt, ...);
>>> +extern void die(int err, const char *fmt, ...) X_FORMAT((printf, 2, 3));
>>>    extern void (*at_die)(void);
>>>      /* exit status - bits below are ORed */
>>> diff --git a/support/include/xlog.h b/support/include/xlog.h
>>> index a11463ed..32ff5a1b 100644
>>> --- a/support/include/xlog.h
>>> +++ b/support/include/xlog.h
>>> @@ -7,6 +7,10 @@
>>>    #ifndef XLOG_H
>>>    #define XLOG_H
>>>    +#ifdef HAVE_CONFIG_H
>>> +#include <config.h>
>>> +#endif
>>> +
>>>    #include <stdarg.h>
>>>      /* These are logged always. L_FATAL also does exit(1) */
>>> @@ -35,6 +39,12 @@ struct xlog_debugfac {
>>>        int        df_fac;
>>>    };
>>>    +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
>>> +#define XLOG_FORMAT(_x) __attribute__((__format__ _x))
>>> +#else
>>> +#define XLOG_FORMAT(_x)
>>> +#endif
>>> +
>>>    extern int export_errno;
>>>    void            xlog_open(char *progname);
>>>    void            xlog_stderr(int on);
>>> @@ -43,10 +53,10 @@ void            xlog_config(int fac, int on);
>>>    void            xlog_sconfig(char *, int on);
>>>    void            xlog_from_conffile(char *);
>>>    int            xlog_enabled(int fac);
>>> -void            xlog(int fac, const char *fmt, ...);
>>> -void            xlog_warn(const char *fmt, ...);
>>> -void            xlog_err(const char *fmt, ...);
>>> -void            xlog_errno(int err, const char *fmt, ...);
>>> -void            xlog_backend(int fac, const char *fmt, va_list args);
>>> +void            xlog(int fac, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
>>> +void            xlog_warn(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
>>> +void            xlog_err(const char *fmt, ...) XLOG_FORMAT((printf, 1, 2));
>>> +void            xlog_errno(int err, const char *fmt, ...) XLOG_FORMAT((printf, 2, 3));
>>> +void            xlog_backend(int fac, const char *fmt, va_list args) XLOG_FORMAT((printf, 2, 0));
>>>      #endif /* XLOG_H */
>>> diff --git a/support/nfs/svc_create.c b/support/nfs/svc_create.c
>>> index 4e14430d..976c2d29 100644
>>> --- a/support/nfs/svc_create.c
>>> +++ b/support/nfs/svc_create.c
>>> @@ -184,7 +184,7 @@ svc_create_sock(const struct sockaddr *sap, socklen_t salen,
>>>            type = SOCK_STREAM;
>>>            break;
>>>        default:
>>> -        xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %u",
>>> +        xlog(D_GENERAL, "%s: Unrecognized bind address semantics: %lu",
>>>                __func__, nconf->nc_semantics);
>>>            return -1;
>>>        }
>>> diff --git a/support/nsm/rpc.c b/support/nsm/rpc.c
>>> index ae49006c..08b4746f 100644
>>> --- a/support/nsm/rpc.c
>>> +++ b/support/nsm/rpc.c
>>> @@ -182,7 +182,7 @@ nsm_xmit_getport(const int sock, const struct sockaddr_in *sin,
>>>        uint32_t xid;
>>>        XDR xdr;
>>>    -    xlog(D_CALL, "Sending PMAP_GETPORT for %u, %u, udp", program, version);
>>> +    xlog(D_CALL, "Sending PMAP_GETPORT for %lu, %lu, udp", program, version);
>>>          nsm_init_xdrmem(msgbuf, NSM_MAXMSGSIZE, &xdr);
>>>        xid = nsm_init_rpc_header(PMAPPROG, PMAPVERS,
>>> diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
>>> index 5cca4175..a04a7898 100644
>>> --- a/utils/exportfs/exportfs.c
>>> +++ b/utils/exportfs/exportfs.c
>>> @@ -651,6 +651,9 @@ out:
>>>        return result;
>>>    }
>>>    +#ifdef HAVE_FUNC_ATTRIBUTE_FORMAT
>>> +__attribute__((format (printf, 2, 3)))
>>> +#endif
>>>    static char
>>>    dumpopt(char c, char *fmt, ...)
>>>    {
>>> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
>>> index e25a4337..3861f84a 100644
>>> --- a/utils/mountd/cache.c
>>> +++ b/utils/mountd/cache.c
>>> @@ -987,8 +987,7 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
>>>                } else if (found_type == i && found->m_warned == 0) {
>>>                    xlog(L_WARNING, "%s exported to both %s and %s, "
>>>                         "arbitrarily choosing options from first",
>>> -                     path, found->m_client->m_hostname, exp->m_client->m_hostname,
>>> -                     dom);
>>> +                     path, found->m_client->m_hostname, exp->m_client->m_hostname);
>>>                    found->m_warned = 1;
>>>                }
>>>            }
>>> diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
>>> index 33571ecb..66366434 100644
>>> --- a/utils/mountd/mountd.c
>>> +++ b/utils/mountd/mountd.c
>>> @@ -210,10 +210,10 @@ killer (int sig)
>>>    }
>>>      static void
>>> -sig_hup (int sig)
>>> +sig_hup (int UNUSED(sig))
>>>    {
>>>        /* don't exit on SIGHUP */
>>> -    xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n", sig);
>>> +    xlog (L_NOTICE, "Received SIGHUP... Ignoring.\n");
>>>        return;
>>>    }
>>>    diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcld/nfsdcld.c
>>> index cbf71fc6..7e894e49 100644
>>> --- a/utils/nfsdcld/nfsdcld.c
>>> +++ b/utils/nfsdcld/nfsdcld.c
>>> @@ -212,7 +212,7 @@ cld_inotify_cb(int UNUSED(fd), short which, void *data)
>>>        default:
>>>            /* anything else is fatal */
>>>            xlog(L_FATAL, "%s: unable to open new pipe (%d). Aborting.",
>>> -            ret, __func__);
>>> +            __func__, ret);
>>>            exit(ret);
>>>        }
>>>    diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcld/sqlite.c
>>> index fa81df87..afa63499 100644
>>> --- a/utils/nfsdcld/sqlite.c
>>> +++ b/utils/nfsdcld/sqlite.c
>>> @@ -473,7 +473,7 @@ sqlite_fix_table_name(const char *name)
>>>        }
>>>        ret = sqlite3_exec(dbh, (const char *)buf, NULL, NULL, &err);
>>>        if (ret != SQLITE_OK) {
>>> -        xlog(L_ERROR, "Unable to fix table for epoch %d: %s",
>>> +        xlog(L_ERROR, "Unable to fix table for epoch %"PRIu64": %s",
>>>                 val, err);
>>>            goto out;
>>>        }
>>> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
>>> index fc00da7a..cf7f65e9 100644
>>> --- a/utils/nfsidmap/nfsidmap.c
>>> +++ b/utils/nfsidmap/nfsidmap.c
>>> @@ -18,7 +18,7 @@
>>>    #include "xcommon.h"
>>>      int verbose = 0;
>>> -char *usage = "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]";
>>> +#define USAGE "Usage: %s [-vh] [-c || [-u|-g|-r key] || -d || -l || [-t timeout] key desc]"
>>>      #define MAX_ID_LEN   11
>>>    #define IDMAP_NAMESZ 128
>>> @@ -403,7 +403,7 @@ int main(int argc, char **argv)
>>>                break;
>>>            case 'h':
>>>            default:
>>> -            xlog_warn(usage, progname);
>>> +            xlog_warn(USAGE, progname);
>>>                exit(opt == 'h' ? 0 : 1);
>>>            }
>>>        }
>>> @@ -435,7 +435,7 @@ int main(int argc, char **argv)
>>>        xlog_stderr(verbose);
>>>        if ((argc - optind) != 2) {
>>>            xlog_warn("Bad arg count. Check /etc/request-key.conf");
>>> -        xlog_warn(usage, progname);
>>> +        xlog_warn(USAGE, progname);
>>>            return EXIT_FAILURE;
>>>        }
>>>    @@ -453,7 +453,7 @@ int main(int argc, char **argv)
>>>            return EXIT_FAILURE;
>>>        }
>>>        if (verbose) {
>>> -        xlog_warn("key: 0x%lx type: %s value: %s timeout %ld",
>>> +        xlog_warn("key: 0x%x type: %s value: %s timeout %d",
>>>                key, type, value, timeout);
>>>        }
>>>    diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
>>> index c4f6364f..5b261480 100644
>>> --- a/utils/statd/rmtcall.c
>>> +++ b/utils/statd/rmtcall.c
>>> @@ -247,7 +247,7 @@ process_reply(FD_SET_TYPE *rfds)
>>>            xlog_warn("%s: service %d not registered on localhost",
>>>                __func__, NL_MY_PROG(lp));
>>>        } else {
>>> -        xlog(D_GENERAL, "%s: Callback to %s (for %d) succeeded",
>>> +        xlog(D_GENERAL, "%s: Callback to %s (for %s) succeeded",
>>>                __func__, NL_MY_NAME(lp), NL_MON_NAME(lp));
>>>        }
>>>        nlist_free(&notify, lp);
>>> diff --git a/utils/statd/statd.c b/utils/statd/statd.c
>>> index 14673800..8eef2ff2 100644
>>> --- a/utils/statd/statd.c
>>> +++ b/utils/statd/statd.c
>>> @@ -136,7 +136,7 @@ static void log_modes(void)
>>>        strcat(buf, "TI-RPC ");
>>>    #endif
>>>    -    xlog_warn(buf);
>>> +    xlog_warn("%s", buf);
>>>    }
>>>      /*
>>> diff --git a/utils/statd/svc_run.c b/utils/statd/svc_run.c
>>> index d1dbd74a..e343c768 100644
>>> --- a/utils/statd/svc_run.c
>>> +++ b/utils/statd/svc_run.c
>>> @@ -53,6 +53,7 @@
>>>      #include <errno.h>
>>>    #include <time.h>
>>> +#include <inttypes.h>
>>>    #include "statd.h"
>>>    #include "notlist.h"
>>>    @@ -104,8 +105,8 @@ my_svc_run(int sockfd)
>>>                  tv.tv_sec  = NL_WHEN(notify) - now;
>>>                tv.tv_usec = 0;
>>> -            xlog(D_GENERAL, "Waiting for reply... (timeo %d)",
>>> -                            tv.tv_sec);
>>> +            xlog(D_GENERAL, "Waiting for reply... (timeo %jd)",
>>> +                            (intmax_t)tv.tv_sec);
>>>                selret = select(FD_SETSIZE, &readfds,
>>>                    (void *) 0, (void *) 0, &tv);
>>>            } else {

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 16:33 [nfs-utils PATCH] Add printf format checking Joshua Watt
2019-09-06 20:24 ` [nfs-utils PATCH v2] " Joshua Watt
2019-09-16 18:33   ` Joshua Watt
2019-09-20 14:05     ` Steve Dickson
2019-09-23 14:04       ` Joshua Watt

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org linux-nfs@archiver.kernel.org
	public-inbox-index linux-nfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox