[PATCH 0/4] [pull] mostly uuidd changes

[PATCH 0/4] [pull] mostly uuidd changes

[Sami Kerola]

Hello,

The changes in this pull are pretty simple apart from one, that replaces use
of old libc interfaces with newer.  That change is a little bit more hairy
than the other, but should work just fine.

FYI if you pull the changes from my repository you will see they are signed
using my gpg key FDE9B739.  See with git log --show-signature.

----------------------------------------------------------------
The following changes since commit fa8945db4aa0526655655526c3ace8fafaf32f8e:
  uuidd: improve socket activation error messaging (2015-10-02 12:38:30 +0200)
are available in the git repository at:
  git://github.com/kerolasa/lelux-utiliteetit.git uuidd
for you to fetch changes up to 35d009dafa4cfa5ef67996e35c6a38525a91f354:
  uuidd: fix shadow declaration (2015-10-03 19:22:58 +0100)
----------------------------------------------------------------

Sami Kerola (4):
  uuidd: slice up the usage text and normalize its layout
  blkid, uuidd, uuidgen: assume getopt.h and getopt.h are available
  uuidd: use signalfd() and setup_timer()
  uuidd: fix shadow declaration

 misc-utils/Makemodule.am |   4 +-
 misc-utils/blkid.c       |   6 --
 misc-utils/uuidd.c       | 189 +++++++++++++++++++++++++++++------------------
 misc-utils/uuidgen.c     |   8 --
 4 files changed, 120 insertions(+), 87 deletions(-)

-- 
2.6.0

[PATCH 1/4] uuidd: slice up the usage text and normalize its layout

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/uuidd.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/misc-utils/uuidd.c b/misc-utils/uuidd.c
index 9d6d08d..76f5e41 100644
--- a/misc-utils/uuidd.c
+++ b/misc-utils/uuidd.c
@@ -69,28 +69,26 @@ struct uuidd_cxt_t {
 static void __attribute__ ((__noreturn__)) usage(FILE * out)
 {
 	fputs(USAGE_HEADER, out);
-	fprintf(out,
-	      _(" %s [options]\n"), program_invocation_short_name);
-
+	fprintf(out, _(" %s [options]\n"), program_invocation_short_name);
 	fputs(USAGE_SEPARATOR, out);
 	fputs(_("A daemon for generating UUIDs.\n"), out);
-
 	fputs(USAGE_OPTIONS, out);
-	fputs(_(" -p, --pid <path>        path to pid file\n"
-		" -s, --socket <path>     path to socket\n"
-		" -T, --timeout <sec>     specify inactivity timeout\n"
-		" -k, --kill              kill running daemon\n"
-		" -r, --random            test random-based generation\n"
-		" -t, --time              test time-based generation\n"
-		" -n, --uuids <num>       request number of uuids\n"
-		" -P, --no-pid            do not create pid file\n"
-		" -F, --no-fork           do not daemonize using double-fork\n"
-		" -S, --socket-activation do not create listening socket\n"
-		" -d, --debug             run in debugging mode\n"
-		" -q, --quiet             turn on quiet mode\n"
-		" -V, --version           output version information and exit\n"
-		" -h, --help              display this help and exit\n\n"), out);
-
+	fputs(_(" -p, --pid <path>        path to pid file\n"), out);
+	fputs(_(" -s, --socket <path>     path to socket\n"), out);
+	fputs(_(" -T, --timeout <sec>     specify inactivity timeout\n"), out);
+	fputs(_(" -k, --kill              kill running daemon\n"), out);
+	fputs(_(" -r, --random            test random-based generation\n"), out);
+	fputs(_(" -t, --time              test time-based generation\n"), out);
+	fputs(_(" -n, --uuids <num>       request number of uuids\n"), out);
+	fputs(_(" -P, --no-pid            do not create pid file\n"), out);
+	fputs(_(" -F, --no-fork           do not daemonize using double-fork\n"), out);
+	fputs(_(" -S, --socket-activation do not create listening socket\n"), out);
+	fputs(_(" -d, --debug             run in debugging mode\n"), out);
+	fputs(_(" -q, --quiet             turn on quiet mode\n"), out);
+	fputs(USAGE_SEPARATOR, out);
+	fputs(USAGE_HELP, out);
+	fputs(USAGE_VERSION, out);
+	fprintf(out, USAGE_MAN_TAIL("uuidd(8)"));
 	exit(out == stderr ? EXIT_FAILURE : EXIT_SUCCESS);
 }
 
-- 
2.6.0

[PATCH 2/4] blkid, uuidd, uuidgen: assume getopt.h and getopt.h are available

[Sami Kerola]

These headers are in use allover this project without issues.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/blkid.c   | 6 ------
 misc-utils/uuidd.c   | 8 --------
 misc-utils/uuidgen.c | 8 --------
 3 files changed, 22 deletions(-)

diff --git a/misc-utils/blkid.c b/misc-utils/blkid.c
index bbe7b62..c0be457 100644
--- a/misc-utils/blkid.c
+++ b/misc-utils/blkid.c
@@ -17,13 +17,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <errno.h>
-#ifdef HAVE_GETOPT_H
 #include <getopt.h>
-#else
-extern int getopt(int argc, char * const argv[], const char *optstring);
-extern char *optarg;
-extern int optind;
-#endif
 
 #define OUTPUT_VALUE_ONLY	(1 << 1)
 #define OUTPUT_DEVICE_ONLY	(1 << 2)
diff --git a/misc-utils/uuidd.c b/misc-utils/uuidd.c
index 76f5e41..e19be52 100644
--- a/misc-utils/uuidd.c
+++ b/misc-utils/uuidd.c
@@ -9,9 +9,7 @@
  * %End-Header%
  */
 #include <stdio.h>
-#ifdef HAVE_STDLIB_H
 #include <stdlib.h>
-#endif
 #include <unistd.h>
 #include <inttypes.h>
 #include <errno.h>
@@ -23,13 +21,7 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <string.h>
-#ifdef HAVE_GETOPT_H
 #include <getopt.h>
-#else
-extern int getopt(int argc, char * const argv[], const char *optstring);
-extern char *optarg;
-extern int optind;
-#endif
 
 #include "uuid.h"
 #include "uuidd.h"
diff --git a/misc-utils/uuidgen.c b/misc-utils/uuidgen.c
index 54c5773..40b00ff 100644
--- a/misc-utils/uuidgen.c
+++ b/misc-utils/uuidgen.c
@@ -10,16 +10,8 @@
  */
 
 #include <stdio.h>
-#ifdef HAVE_STDLIB_H
 #include <stdlib.h>
-#endif
-#ifdef HAVE_GETOPT_H
 #include <getopt.h>
-#else
-extern int getopt(int argc, char * const argv[], const char *optstring);
-extern char *optarg;
-extern int optind;
-#endif
 
 #include "uuid.h"
 #include "nls.h"
-- 
2.6.0

[PATCH 3/4] uuidd: use signalfd() and setup_timer()

[Sami Kerola]

Point of this change is to replace use of signal() and alarm() system calls
using newer interfaces.  Nice side effect is that the point where timer was
earlier used cannot be distracted by sending rogue SIGALRM.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/Makemodule.am |   4 +-
 misc-utils/uuidd.c       | 135 +++++++++++++++++++++++++++++++++--------------
 2 files changed, 98 insertions(+), 41 deletions(-)

diff --git a/misc-utils/Makemodule.am b/misc-utils/Makemodule.am
index e017952..bc1c346 100644
--- a/misc-utils/Makemodule.am
+++ b/misc-utils/Makemodule.am
@@ -90,10 +90,10 @@ endif
 if BUILD_UUIDD
 usrsbin_exec_PROGRAMS += uuidd
 dist_man_MANS += misc-utils/uuidd.8
-uuidd_LDADD = $(LDADD) libuuid.la libcommon.la
+uuidd_LDADD = $(LDADD) libuuid.la libcommon.la -lrt
 uuidd_CFLAGS = $(DAEMON_CFLAGS) $(AM_CFLAGS) -I$(ul_libuuid_incdir)
 uuidd_LDFLAGS = $(DAEMON_LDFLAGS) $(AM_LDFLAGS)
-uuidd_SOURCES = misc-utils/uuidd.c
+uuidd_SOURCES = misc-utils/uuidd.c lib/monotonic.c lib/timer.c
 if HAVE_SYSTEMD
 uuidd_LDADD += $(SYSTEMD_LIBS) $(SYSTEMD_DAEMON_LIBS)
 uuidd_CFLAGS += $(SYSTEMD_CFLAGS) $(SYSTEMD_DAEMON_CFLAGS)
diff --git a/misc-utils/uuidd.c b/misc-utils/uuidd.c
index e19be52..c897372 100644
--- a/misc-utils/uuidd.c
+++ b/misc-utils/uuidd.c
@@ -22,6 +22,8 @@
 #include <signal.h>
 #include <string.h>
 #include <getopt.h>
+#include <sys/signalfd.h>
+#include <poll.h>
 
 #include "uuid.h"
 #include "uuidd.h"
@@ -30,6 +32,8 @@
 #include "closestream.h"
 #include "strutils.h"
 #include "optutils.h"
+#include "monotonic.h"
+#include "timer.h"
 
 #ifdef HAVE_LIBSYSTEMD
 # include <systemd/sd-daemon.h>
@@ -51,6 +55,8 @@
 
 /* server loop control structure */
 struct uuidd_cxt_t {
+	const char	*cleanup_pidfile;
+	const char	*cleanup_socket;
 	uint32_t	timeout;
 	unsigned int	debug: 1,
 			quiet: 1,
@@ -96,17 +102,6 @@ static void create_daemon(void)
 		err(EXIT_FAILURE, "setreuid");
 }
 
-static const char *cleanup_pidfile, *cleanup_socket;
-
-static void terminate_intr(int signo CODE_ATTR((unused)))
-{
-	if (cleanup_pidfile)
-		unlink(cleanup_pidfile);
-	if (cleanup_socket)
-		unlink(cleanup_socket);
-	exit(EXIT_SUCCESS);
-}
-
 static int call_daemon(const char *socket_path, int op, char *buf,
 		       size_t buflen, int *num, const char **err_context)
 {
@@ -199,12 +194,10 @@ static int call_daemon(const char *socket_path, int op, char *buf,
 /*
  * Exclusively create and open a pid file with path @pidfile_path
  *
- * Set cleanup_pidfile global variable for the cleanup
- * handler. @pidfile_path must not be NULL.
- *
  * Return file descriptor of the created pid_file.
  */
-static int create_pidfile(const char *pidfile_path, int quiet)
+static int create_pidfile(struct uuidd_cxt_t *uuidd_cxt,
+			  const char *pidfile_path, int quiet)
 {
 	int		fd_pidfile;
 	struct flock	fl;
@@ -215,7 +208,7 @@ static int create_pidfile(const char *pidfile_path, int quiet)
 			warn(_("cannot open %s"), pidfile_path);
 		exit(EXIT_FAILURE);
 	}
-	cleanup_pidfile = pidfile_path;
+	uuidd_cxt->cleanup_pidfile = pidfile_path;
 
 	fl.l_type = F_WRLCK;
 	fl.l_whence = SEEK_SET;
@@ -242,14 +235,15 @@ static int create_pidfile(const char *pidfile_path, int quiet)
  *
  * Return file descriptor corresponding to created socket.
  */
-static int create_socket(const char *socket_path, int will_fork, int quiet)
+static int create_socket(struct uuidd_cxt_t *uuidd_cxt,
+			 const char *socket_path, int will_fork)
 {
 	struct sockaddr_un	my_addr;
 	mode_t			save_umask;
 	int 			s;
 
 	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
-		if (!quiet)
+		if (!uuidd_cxt->quiet)
 			warn(_("couldn't create unix stream socket"));
 		exit(EXIT_FAILURE);
 	}
@@ -274,18 +268,52 @@ static int create_socket(const char *socket_path, int will_fork, int quiet)
 	save_umask = umask(0);
 	if (bind(s, (const struct sockaddr *) &my_addr,
 		 sizeof(struct sockaddr_un)) < 0) {
-		if (!quiet)
+		if (!uuidd_cxt->quiet)
 			warn(_("couldn't bind unix socket %s"), socket_path);
 		exit(EXIT_FAILURE);
 	}
 	umask(save_umask);
-	cleanup_socket = socket_path;
+	uuidd_cxt->cleanup_socket = socket_path;
 
 	return s;
 }
 
+static void __attribute__((__noreturn__)) all_done(const struct uuidd_cxt_t *uuidd_cxt, int ret)
+{
+	if (uuidd_cxt->cleanup_pidfile)
+		unlink(uuidd_cxt->cleanup_pidfile);
+	if (uuidd_cxt->cleanup_socket)
+		unlink(uuidd_cxt->cleanup_socket);
+	exit(ret);
+}
+
+static void handle_signal(const struct uuidd_cxt_t *uuidd_cxt, int fd)
+{
+	struct signalfd_siginfo info;
+	ssize_t bytes;
+
+	bytes = read(fd, &info, sizeof(info));
+	if (bytes != sizeof(info)) {
+		if (errno == EAGAIN)
+			return;
+		warn(_("receiving signal failed"));
+		info.ssi_signo = 0;
+	}
+	if (info.ssi_signo == SIGPIPE)
+		return;		/* ignored */
+	all_done(uuidd_cxt, EXIT_SUCCESS);
+}
+
+static void timeout_handler(int sig __attribute__((__unused__)),
+			    siginfo_t * info,
+			    void *context __attribute__((__unused__)))
+{
+	if (info->si_code == SI_TIMER)
+		errx(EXIT_FAILURE, _("timed out"));
+}
+
 static void server_loop(const char *socket_path, const char *pidfile_path,
-			const struct uuidd_cxt_t *uuidd_cxt)
+			struct uuidd_cxt_t *uuidd_cxt)
 {
 	struct sockaddr_un	from_addr;
 	socklen_t		fromlen;
@@ -297,30 +325,39 @@ static void server_loop(const char *socket_path, const char *pidfile_path,
 	int			s = 0;
 	int			fd_pidfile = -1;
 	int			ret;
+	struct pollfd 		pfd[2];
+	sigset_t		sigmask;
+	int			sigfd;
+	enum {
+				POLLFD_SIGNAL = 0,
+				POLLFD_SOCKET
+	};
 
 #ifdef HAVE_LIBSYSTEMD
 	if (!uuidd_cxt->no_sock)	/* no_sock implies no_fork and no_pid */
 #endif
 	{
+		static timer_t t_id;
+		struct itimerval timeout;
 
-		signal(SIGALRM, terminate_intr);
-		alarm(30);
+		memset(&timeout, 0, sizeof timeout);
+		timeout.it_value.tv_sec = 30;
+		if (setup_timer(&t_id, &timeout, &timeout_handler))
+			err(EXIT_FAILURE, _("cannot not setup timer"));
 		if (pidfile_path)
-			fd_pidfile = create_pidfile(pidfile_path, uuidd_cxt->quiet);
-
+			fd_pidfile = create_pidfile(uuidd_cxt, pidfile_path, uuidd_cxt->quiet);
 		ret = call_daemon(socket_path, UUIDD_OP_GETPID, reply_buf,
 				  sizeof(reply_buf), 0, NULL);
+		cancel_timer(&t_id);
 		if (ret > 0) {
 			if (!uuidd_cxt->quiet)
 				warnx(_("uuidd daemon is already running at pid %s"),
-				       reply_buf);
+					reply_buf);
 			exit(EXIT_FAILURE);
 		}
-		alarm(0);
 
-		s = create_socket(socket_path,
-				  (!uuidd_cxt->debug || !uuidd_cxt->no_fork),
-				  uuidd_cxt->quiet);
+		s = create_socket(uuidd_cxt, socket_path,
+				  (!uuidd_cxt->debug || !uuidd_cxt->no_fork));
 		if (listen(s, SOMAXCONN) < 0) {
 			if (!uuidd_cxt->quiet)
 				warn(_("couldn't listen on unix socket %s"), socket_path);
@@ -342,12 +379,6 @@ static void server_loop(const char *socket_path, const char *pidfile_path,
 
 	}
 
-	signal(SIGHUP, terminate_intr);
-	signal(SIGINT, terminate_intr);
-	signal(SIGTERM, terminate_intr);
-	signal(SIGALRM, terminate_intr);
-	signal(SIGPIPE, SIG_IGN);
-
 #ifdef HAVE_LIBSYSTEMD
 	if (uuidd_cxt->no_sock) {
 		const int ret = sd_listen_fds(0);
@@ -365,12 +396,38 @@ static void server_loop(const char *socket_path, const char *pidfile_path,
 	}
 #endif
 
+	sigemptyset(&sigmask);
+	sigaddset(&sigmask, SIGHUP);
+	sigaddset(&sigmask, SIGINT);
+	sigaddset(&sigmask, SIGTERM);
+	sigaddset(&sigmask, SIGALRM);
+	sigaddset(&sigmask, SIGPIPE);
+	/* Block signals so that they aren't handled according to their
+	 * default dispositions */
+	sigprocmask(SIG_BLOCK, &sigmask, NULL);
+	if ((sigfd = signalfd(-1, &sigmask, 0)) < 0)
+		err(EXIT_FAILURE, _("cannot set signal handler"));
+
+	pfd[POLLFD_SIGNAL].fd = sigfd;
+	pfd[POLLFD_SOCKET].fd = s;
+	pfd[POLLFD_SIGNAL].events = pfd[POLLFD_SOCKET].events = POLLIN | POLLERR | POLLHUP;
+
 	while (1) {
+		ret = poll(pfd, ARRAY_SIZE(pfd), (uuidd_cxt->timeout == 0 ? -1 : uuidd_cxt->timeout * 1000));
+		if (ret < 0) {
+			if (errno == EAGAIN)
+				continue;
+			warn(_("poll failed"));
+				all_done(uuidd_cxt, EXIT_FAILURE);
+		}
+		if (ret == 0)		/* truen when poll() times out */
+			all_done(uuidd_cxt, EXIT_SUCCESS);
+		if (pfd[POLLFD_SIGNAL].revents != 0)
+			handle_signal(uuidd_cxt, sigfd);
+		if (pfd[POLLFD_SOCKET].revents == 0)
+			continue;
 		fromlen = sizeof(from_addr);
-		if (uuidd_cxt->timeout != 0)
-			alarm(uuidd_cxt->timeout);
 		ns = accept(s, (struct sockaddr *) &from_addr, &fromlen);
-		alarm(0);
 		if (ns < 0) {
 			if ((errno == EAGAIN) || (errno == EINTR))
 				continue;
-- 
2.6.0

[PATCH 4/4] uuidd: fix shadow declaration

[Sami Kerola]

misc-utils/uuidd.c:384:13: warning: declaration of 'ret' shadows a previous
			   local [-Wshadow]
misc-utils/uuidd.c:327:6: note: shadowed declaration is here

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/uuidd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/misc-utils/uuidd.c b/misc-utils/uuidd.c
index c897372..2359f98 100644
--- a/misc-utils/uuidd.c
+++ b/misc-utils/uuidd.c
@@ -381,15 +381,15 @@ static void server_loop(const char *socket_path, const char *pidfile_path,
 
 #ifdef HAVE_LIBSYSTEMD
 	if (uuidd_cxt->no_sock) {
-		const int ret = sd_listen_fds(0);
+		const int r = sd_listen_fds(0);
 
-		if (ret < 0) {
-			errno = ret * -1;
+		if (r < 0) {
+			errno = r * -1;
 			err(EXIT_FAILURE, _("sd_listen_fds() failed"));
-		} else if (ret == 0)
+		} else if (r == 0)
 			errx(EXIT_FAILURE,
 			     _("no file descriptors received, check systemctl status uuidd.socket"));
-		else if (1 < ret)
+		else if (1 < r)
 			errx(EXIT_FAILURE,
 			     _("too many file descriptors received, check uuidd.socket"));
 		s = SD_LISTEN_FDS_START + 0;
-- 
2.6.0

Re: [PATCH 0/4] [pull] mostly uuidd changes

[Karel Zak]

On Sat, Oct 03, 2015 at 07:31:03PM +0100, Sami Kerola wrote:
>   uuidd: slice up the usage text and normalize its layout
>   blkid, uuidd, uuidgen: assume getopt.h and getopt.h are available
>   uuidd: use signalfd() and setup_timer()
>   uuidd: fix shadow declaration

 Applied, thanks! 

 I see we don't have regression tests for uuidd. It would be nice to
 have something before release ;-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com