changeset 944:057fcc76587f draft

As our logger calls emulate syslog, we can use %m. However, this means we have to convert it for printf as POSIX does not require that printf supports it. This also means we drop -pedantic from our GCC flags.
author Roy Marples <roy@marples.name>
date Fri, 05 Sep 2008 13:12:35 +0000
parents 27623db51ce3
children cd4fa21cbb60
files arp.c bind.c common.c config.h configure.c dhcpcd.c if-options.c logger.c logger.h mk/cc.mk
diffstat 10 files changed, 117 insertions(+), 61 deletions(-) [+]
line wrap: on
line diff
--- a/arp.c	Fri Sep 05 11:54:44 2008 +0000
+++ b/arp.c	Fri Sep 05 13:12:35 2008 +0000
@@ -43,11 +43,10 @@
 #define ARP_LEN \
 	(sizeof(struct arphdr) + (2 * sizeof(uint32_t)) + (2 * HWADDR_LEN))
 
-static uint8_t arp_buffer[ARP_LEN];
-
 static int
 send_arp(const struct interface *iface, int op, in_addr_t sip, in_addr_t tip)
 {
+	uint8_t arp_buffer[ARP_LEN];
 	struct arphdr ar;
 	size_t len;
 	uint8_t *p;
@@ -91,6 +90,7 @@
 handle_arp_packet(void *arg)
 {
 	struct interface *iface = arg;
+	uint8_t arp_buffer[ARP_LEN];
 	struct arphdr ar;
 	uint32_t reply_s;
 	uint32_t reply_t;
@@ -179,7 +179,7 @@
 		       iface->name, state->claims, ANNOUNCE_NUM);
 	if (send_arp(iface, ARPOP_REQUEST,
 		     state->new->yiaddr, state->new->yiaddr) == -1)
-		logger(LOG_ERR, "send_arp: %s", strerror(errno));
+		logger(LOG_ERR, "send_arp: %m");
 	if (state->claims < ANNOUNCE_NUM) {
 		add_timeout_sec(ANNOUNCE_WAIT, send_arp_announce, iface);
 		return;
@@ -237,6 +237,6 @@
 		"%s: sending ARP probe (%d of %d), next in %0.2f seconds",
 		iface->name, state->probes, PROBE_NUM,  timeval_to_double(&tv));
 	if (send_arp(iface, ARPOP_REQUEST, 0, state->offer->yiaddr) == -1)
-		logger(LOG_ERR, "send_arp: %s", strerror(errno));
+		logger(LOG_ERR, "send_arp: %m");
 }
 
--- a/bind.c	Fri Sep 05 11:54:44 2008 +0000
+++ b/bind.c	Fri Sep 05 13:12:35 2008 +0000
@@ -25,7 +25,6 @@
  * SUCH DAMAGE.
  */
 
-#include <errno.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -58,13 +57,13 @@
 	sigprocmask(SIG_SETMASK, &full, &old);
 	/* Setup a signal pipe so parent knows when to exit. */
 	if (pipe(sidpipe) == -1) {
-		logger(LOG_ERR, "pipe: %s", strerror(errno));
+		logger(LOG_ERR, "pipe: %m");
 		return -1;
 	}
 	logger(LOG_INFO, "forking to background");
 	switch (pid = fork()) {
 		case -1:
-			logger(LOG_ERR, "fork: %s", strerror(errno));
+			logger(LOG_ERR, "fork: %m");
 			exit(EXIT_FAILURE);
 			/* NOTREACHED */
 		case 0:
--- a/common.c	Fri Sep 05 11:54:44 2008 +0000
+++ b/common.c	Fri Sep 05 13:12:35 2008 +0000
@@ -33,7 +33,6 @@
 #include <sys/param.h>
 #include <sys/time.h>
 
-#include <errno.h>
 #include <fcntl.h>
 #ifdef BSD
 #  include <paths.h>
@@ -164,7 +163,7 @@
 	if ((flags = fcntl(fd, F_GETFD, 0)) == -1
 	    || fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1)
 	{
-		logger(LOG_ERR, "fcntl: %s", strerror(errno));
+		logger(LOG_ERR, "fcntl: %m");
 		return -1;
 	}
 	return 0;
@@ -178,7 +177,7 @@
 	if ((flags = fcntl(fd, F_GETFL, 0)) == -1
 	    || fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1)
 	{
-		logger(LOG_ERR, "fcntl: %s", strerror(errno));
+		logger(LOG_ERR, "fcntl: %m");
 		return -1;
 	}
 	return 0;
--- a/config.h	Fri Sep 05 11:54:44 2008 +0000
+++ b/config.h	Fri Sep 05 13:12:35 2008 +0000
@@ -37,7 +37,7 @@
  * Ideally the host network scripts should add the link local route for us.
  * If not, you can define this to get dhcpcd to always add the link local route.
  */
-// #define IPV4LL_ALWAYSROUTE 
+/* #define IPV4LL_ALWAYSROUTE */
 
 /* Some systems do not have a working fork. */
 /* #define THERE_IS_NO_FORK */
--- a/configure.c	Fri Sep 05 11:54:44 2008 +0000
+++ b/configure.c	Fri Sep 05 13:12:35 2008 +0000
@@ -62,12 +62,12 @@
 
 	switch (pid = vfork()) {
 	case -1:
-		logger(LOG_ERR, "vfork: %s", strerror(errno));
+		logger(LOG_ERR, "vfork: %m");
 		break;
 	case 0:
 		sigprocmask(SIG_SETMASK, &old, NULL);
 		execve(argv[0], argv, env);
-		logger(LOG_ERR, "%s: %s", argv[0], strerror(errno));
+		logger(LOG_ERR, "%s: %m", argv[0]);
 		_exit(127);
 		/* NOTREACHED */
 	}
@@ -151,7 +151,7 @@
 		/* Wait for the script to finish */
 		while (waitpid(pid, &status, 0) == -1) {
 			if (errno != EINTR) {
-				logger(LOG_ERR, "waitpid: %s", strerror(errno));
+				logger(LOG_ERR, "waitpid: %m");
 				status = -1;
 				break;
 			}
@@ -178,7 +178,7 @@
 	free(addr);
 	retval = del_route(iface, &rt->dest, &rt->net, &rt->gate, metric);
 	if (retval != 0 && errno != ENOENT && errno != ESRCH)
-		logger(LOG_ERR," del_route: %s", strerror(errno));
+		logger(LOG_ERR," del_route: %m");
 	return retval;
 }
 
@@ -278,8 +278,7 @@
 		   ourselves. If so, remember it again. */
 		if (remember < 0) {
 			if (errno != EEXIST)
-				logger(LOG_ERR, "add_route: %s",
-				       strerror(errno));
+				logger(LOG_ERR, "add_route: %m");
 			if (in_routes(iface->routes, rt) == 0)
 				remember = 1;
 		}
@@ -308,7 +307,7 @@
 	       inet_ntocidr(iface->net));
 	retval = del_address(iface, &iface->addr, &iface->net);
 	if (retval == -1 && errno != EADDRNOTAVAIL) 
-		logger(LOG_ERR, "del_address: %s", strerror(errno));
+		logger(LOG_ERR, "del_address: %m");
 	iface->addr.s_addr = 0;
 	iface->net.s_addr = 0;
 	return retval;
@@ -356,7 +355,7 @@
 		if (add_address(iface, &addr, &net, &brd) == -1 &&
 		    errno != EEXIST)
 		{
-			logger(LOG_ERR, "add_address: %s", strerror(errno));
+			logger(LOG_ERR, "add_address: %m");
 			return -1;
 		}
 	}
@@ -396,7 +395,7 @@
 
 	if (!iface->state->lease.frominfo)
 		if (write_lease(iface, dhcp) == -1)
-			logger(LOG_ERR, "write_lease: %s", strerror(errno));
+			logger(LOG_ERR, "write_lease: %m");
 
 	run_script(iface, reason);
 	return 0;
--- a/dhcpcd.c	Fri Sep 05 11:54:44 2008 +0000
+++ b/dhcpcd.c	Fri Sep 05 13:12:35 2008 +0000
@@ -145,8 +145,7 @@
 	if (pidfd > -1) {
 		if (options & DHCPCD_MASTER) {
 			if (stop_control() == -1)
-				logger(LOG_ERR, "stop_control: %s",
-				       strerror(errno));
+				logger(LOG_ERR, "stop_control: %m");
 		}
 		close(pidfd);
 		unlink(pidfile);
@@ -263,15 +262,13 @@
 	if (to.s_addr && to.s_addr != INADDR_BROADCAST) {
 		r = send_packet(iface, to, (uint8_t *)dhcp, len);
 		if (r == -1)
-			logger(LOG_ERR, "%s: send_packet: %s",
-			       iface->name, strerror(errno));
+			logger(LOG_ERR, "%s: send_packet: %m", iface->name);
 	} else {
 		len = make_udp_packet(&udp, (uint8_t *)dhcp, len, from, to);
 		r = send_raw_packet(iface, ETHERTYPE_IP, udp, len);
 		free(udp);
 		if (r == -1)
-			logger(LOG_ERR, "%s: send_raw_packet: %s",
-			       iface->name, strerror(errno));
+			logger(LOG_ERR, "%s: send_raw_packet: %m", iface->name);
 	}
 	free(dhcp);
 	if (r == -1) {
@@ -587,11 +584,11 @@
 		close(iface->udp_fd);
 	if (open_udp_socket(iface) == -1 &&
 	    (errno != EADDRINUSE || iface->addr.s_addr != 0))
-		logger(LOG_ERR, "open_udp_socket: %s", strerror(errno));
+		logger(LOG_ERR, "%s: open_udp_socket: %m", iface->name);
 	if (iface->raw_fd != -1)
 		delete_event(iface->raw_fd);
 	if (open_socket(iface, ETHERTYPE_IP) == -1)
-		logger(LOG_ERR, "open_socket: %s", strerror(errno));
+		logger(LOG_ERR, "%s: open_socket: %m", iface->name);
 	if (iface->raw_fd != -1)
 		add_event(iface->raw_fd, handle_dhcp_packet, iface);
 }
@@ -608,7 +605,7 @@
 		return;
 	switch (carrier_status(iface->name)) {
 	case -1:
-		logger(LOG_ERR, "carrier_status: %s", strerror(errno));
+		logger(LOG_ERR, "carrier_status: %m");
 		break;
 	case 0:
 		if (iface->state->carrier != LINK_DOWN) {
@@ -717,7 +714,7 @@
 		if (ifo->options & DHCPCD_DUID) {
 			duid = xmalloc(DUID_LEN);
 			if ((len = get_duid(duid, iface)) == 0)
-				logger(LOG_ERR, "get_duid: %s", strerror(errno));
+				logger(LOG_ERR, "get_duid: %m");
 		}
 		if (len > 0) {
 			iface->clientid = xmalloc(len + 6);
@@ -834,7 +831,7 @@
 			handle_carrier,
 			handle_new_interface,
 			handle_remove_interface) == -1)
-		logger(LOG_ERR, "manage_link: %s", strerror(errno));
+		logger(LOG_ERR, "manage_link: %m");
 }
 
 static void
@@ -973,6 +970,7 @@
 	setlinebuf(stdout);
 	openlog(PACKAGE, LOG_PID, LOG_LOCAL0);
 	strlcpy(cffile, CONFIG, sizeof(cffile));
+	options = DHCPCD_DAEMONISE;
 
 	/* Test for --help and --version */
 	if (argc > 1) {
@@ -985,8 +983,6 @@
 		}
 	}
 
-	options = DHCPCD_DAEMONISE;
-
 	while ((opt = getopt_long(argc, argv, IF_OPTS, cf_options, &oi)) != -1)
 	{
 		switch (opt) {
@@ -1105,15 +1101,13 @@
 
 		pidfd = open(pidfile, O_WRONLY | O_CREAT | O_NONBLOCK, 0664);
 		if (pidfd == -1) {
-			logger(LOG_ERR, "open `%s': %s",
-			       pidfile, strerror(errno));
+			logger(LOG_ERR, "open `%s': %m", pidfile);
 			exit(EXIT_FAILURE);
 		}
 		/* Lock the file so that only one instance of dhcpcd runs
 		 * on an interface */
 		if (flock(pidfd, LOCK_EX | LOCK_NB) == -1) {
-			logger(LOG_ERR, "flock `%s': %s",
-			       pidfile, strerror(errno));
+			logger(LOG_ERR, "flock `%s': %m", pidfile);
 			exit(EXIT_FAILURE);
 		}
 		if (set_cloexec(pidfd) == -1)
@@ -1131,7 +1125,7 @@
 
 	if (options & DHCPCD_MASTER) {
 		if (start_control() == -1) {
-			logger(LOG_ERR, "start_control: %s", strerror(errno));
+			logger(LOG_ERR, "start_control: %m");
 			exit(EXIT_FAILURE);
 		}
 	}
@@ -1139,8 +1133,7 @@
 	if (ifo->options & DHCPCD_LINK) {
 		linkfd = open_link_socket();
 		if (linkfd == -1)
-			logger(LOG_ERR, "open_link_socket: %s",
-					strerror(errno));
+			logger(LOG_ERR, "open_link_socket: %m");
 		else
 			add_event(linkfd, handle_link, NULL);
 	}
--- a/if-options.c	Fri Sep 05 11:54:44 2008 +0000
+++ b/if-options.c	Fri Sep 05 13:12:35 2008 +0000
@@ -275,7 +275,7 @@
 		else
 			s = 0;
 		if (s == -1) {
-			logger(LOG_ERR, "hostname: %s", strerror(errno));
+			logger(LOG_ERR, "hostname: %m");
 			return -1;
 		}
 		if (s != 0 && ifo->hostname[1] == '.') {
@@ -291,7 +291,7 @@
 		else
 			s = 0;
 		if (s == -1) {
-			logger(LOG_ERR, "vendorclassid: %s", strerror(errno));
+			logger(LOG_ERR, "vendorclassid: %m");
 			return -1;
 		}
 		*ifo->vendorclassid = (uint8_t)s;
@@ -372,7 +372,7 @@
 		s = parse_string((char *)ifo->userclass + ifo->userclass[0] + 2,
 				 s, arg);
 		if (s == -1) {
-			logger(LOG_ERR, "userclass: %s", strerror(errno));
+			logger(LOG_ERR, "userclass: %m");
 			return -1;
 		}
 		if (s != 0) {
@@ -407,7 +407,7 @@
 					 s, arg);
 		}
 		if (s == -1) {
-			logger(LOG_ERR, "vendor: %s", strerror(errno));
+			logger(LOG_ERR, "vendor: %m");
 			return -1;
 		}
 		if (s != 0) {
@@ -467,7 +467,7 @@
 		else
 			s = 0;
 		if (s == -1) {
-			logger(LOG_ERR, "clientid: %s", strerror(errno));
+			logger(LOG_ERR, "clientid: %m");
 			return -1;
 		}
 		ifo->clientid[0] = (uint8_t)s;
--- a/logger.c	Fri Sep 05 11:54:44 2008 +0000
+++ b/logger.c	Fri Sep 05 13:12:35 2008 +0000
@@ -26,6 +26,7 @@
  */
 
 #include <ctype.h>
+#include <errno.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -33,8 +34,26 @@
 #include <syslog.h>
 
 #include "common.h"
+#include "dhcpcd.h"
+#include "if-options.h"
 #include "logger.h"
 
+/* For printf implementation that lack %m conversion in printf.
+ * uClibc does support it, but it's not enabled by default. */
+#ifndef HAVE_PRINTF_M
+# ifdef __GLIBC__
+#  if !defined(__UCLIBC__) && !defined (__dietlibc__)
+#   define HAVE_PRINT_M 1
+#  endif
+# endif
+# ifndef HAVE_PRINT_M
+#  define HAVE_PRINT_M 0
+# endif
+#endif
+
+/* Mac length of format string when we don't have printf with %m */
+#define FMT_LEN 1024
+
 static int loglevel = LOG_INFO;
 
 void
@@ -46,17 +65,55 @@
 void
 logger(int level, const char *fmt, ...)
 {
-	va_list p, p2;
+	va_list va1, va2;
 	FILE *f = stderr;
+#if HAVE_PRINTF_M
+#else
+	char fm[FMT_LEN];
+	char *fp, *e = NULL, *ep;
+	const char *p;
+	size_t el = 0, fl = sizeof(fm);
+#endif
 
-	va_start(p, fmt);
-	va_copy(p2, p);
-	if (level <= LOG_ERR || level <= loglevel) {
-		vfprintf(f, fmt, p);
+	va_start(va1, fmt);
+	va_copy(va2, va1);
+	if (!(options & DHCPCD_DAEMONISED) &&
+	    (level <= LOG_ERR || level <= loglevel))
+	{
+#if HAVE_PRINTF_M
+		vfprintf(f, fmt, va1);
+#else
+		for (p = fmt, fp = fm; *p; p++) {
+			if (*p == '%' && p[1] == 'm') {
+				if (!e) {
+					e = strerror(errno);
+					el = strlen(e);
+				}
+				ep = e;
+				while (fl && *ep) {
+					*fp++ = *ep++;
+					fl--;
+				}
+				p++;
+			} else if (*p == '%' && p[1] == '%' && fl > 2) {
+				*fp++ = '%';
+				*fp++ = '%';
+				p++;
+				fl -= 2;
+			} else {
+				if (fl > 1) {
+					*fp++ = *p;
+					fl--;
+				}
+			}
+		}
+		*fp = '\0';
+		vfprintf(f, fm, va1);
+#endif
 		fputc('\n', f);
 	}
 	if (level < LOG_DEBUG || level <= loglevel)
-		vsyslog(level, fmt, p2);
-	va_end(p2);
-	va_end(p);
+		vsyslog(level, fmt, va2);
+	va_end(va2);
+	va_end(va1);
 }
--- a/logger.h	Fri Sep 05 11:54:44 2008 +0000
+++ b/logger.h	Fri Sep 05 13:12:35 2008 +0000
@@ -28,15 +28,21 @@
 #ifndef LOGGER_H
 #define LOGGER_H
 
+/* We use %m in our logger strings to signal error message.
+ * POSIX allows this in the syslog function.
+ * We also use this GCC attribute to ensure parameters match the format.
+ * However, printf POSIX does not define %m (only glibc does).
+ * This state of affairs will give you a big warning about this if
+ * you use the -pedantic GCC option. It is safe to ignore it. */
 #if defined(__GNUC__)
-#  define _PRINTF_LIKE(_one, _two)  __attribute__ ((__format__ (__printf__, _one, _two)))
+#  define _printf_like(_one, _two) __attribute__((__format__(__printf__, _one, _two)))
 #else
-#  define _PRINTF_LIKE(_one, _two)
+#  define _printf_like(_one, _two)
 #endif
 
 #include <syslog.h>
 
 void setloglevel(int);
-void logger(int, const char *, ...) _PRINTF_LIKE (2, 3);
+void logger(int, const char *, ...) _printf_like(2, 3);
 
 #endif
--- a/mk/cc.mk	Fri Sep 05 11:54:44 2008 +0000
+++ b/mk/cc.mk	Fri Sep 05 13:12:35 2008 +0000
@@ -10,11 +10,14 @@
 CFLAGS+=	${_CSTD}$(shell ${_CSTD_SH})
 
 # Try and use some good cc flags if we're building from git
-_CCFLAGS=	-pedantic -Wall -Wunused -Wimplicit -Wshadow -Wformat=2 \
-		-Wmissing-declarations -Wno-missing-prototypes -Wwrite-strings \
-		-Wbad-function-cast -Wnested-externs -Wcomment -Winline \
-		-Wchar-subscripts -Wcast-align -Wno-format-nonliteral \
-		-Wdeclaration-after-statement -Wsequence-point -Wextra
+# We don't use -pedantic as it will warn about our perfectly valid
+# use of %m in our logger.
+_CCFLAGS=	-Wall -Wextra -Wimplicit -Wshadow -Wformat=2 \
+		-Wmissing-prototypes -Wmissing-declarations \
+		-Wmissing-noreturn -Wmissing-format-attribute \
+		-Wredundant-decls  -Wnested-externs \
+		-Winline -Wwrite-strings -Wcast-align -Wcast-qual -Wpointer-arith \
+		-Wdeclaration-after-statement -Wsequence-point
 _CC_FLAGS_SH=	if ! test -d .git; then echo ""; else for f in ${_CCFLAGS}; do \
 		if ${CC} $$f -S -o /dev/null -xc /dev/null >/dev/null 2>&1; \
 		then printf "%s" "$$f "; fi \