Mark the event_err() functions as __attribute__((noreturn))

This attribute tells gcc (and anything else that understands gcc
attributes) that the functions will never return control, and helps
the optimizer a little.  With luck, it will also tell
less-than-full-program dataflow analysis tools that they don't need to
worry about any code path that involves calling one of these functions
and then returning.

This patch also forces event_exit() to always exit, no matter what the
user-supplied fatal_callback does.  This means that the old unit tests
for the event_err* functions don't work any more, since they assume it
is safe to call event_err* if you've given it a bogus fatal_callback
that doesn't exit.  Instead, we have to make the unit tests fork
before calling event_err(), and have the main unit test process wait
for the event_err() test to exit with a sane exit code.  On unix,
that's trivial.  On windows, let's not bother and just assume that
event_err* works.

diff --git a/log-internal.h b/log-internal.h
index 786a92c78b0e58b481ed0bd0f19b5efa83d4c535..3545da9da5cad7455e29268d20a6b4e11f23f7a5 100644 (file)
--- a/log-internal.h
+++ b/log-internal.h
@@ -31,17 +31,19 @@
 
 #ifdef __GNUC__
 #define EV_CHECK_FMT(a,b) __attribute__((format(printf, a, b)))
+#define EV_NORETURN __attribute__((noreturn))
 #else
 #define EV_CHECK_FMT(a,b)
+#define EV_NORETURN
 #endif
 
 #define _EVENT_ERR_ABORT 0xdeaddead
 
-void event_err(int eval, const char *fmt, ...) EV_CHECK_FMT(2,3);
+void event_err(int eval, const char *fmt, ...) EV_CHECK_FMT(2,3) EV_NORETURN;
 void event_warn(const char *fmt, ...) EV_CHECK_FMT(1,2);
-void event_sock_err(int eval, evutil_socket_t sock, const char *fmt, ...) EV_CHECK_FMT(3,4);
+void event_sock_err(int eval, evutil_socket_t sock, const char *fmt, ...) EV_CHECK_FMT(3,4) EV_NORETURN;
 void event_sock_warn(evutil_socket_t sock, const char *fmt, ...) EV_CHECK_FMT(2,3);
-void event_errx(int eval, const char *fmt, ...) EV_CHECK_FMT(2,3);
+void event_errx(int eval, const char *fmt, ...) EV_CHECK_FMT(2,3) EV_NORETURN;
 void event_warnx(const char *fmt, ...) EV_CHECK_FMT(1,2);
 void event_msgx(const char *fmt, ...) EV_CHECK_FMT(1,2);
 void _event_debugx(const char *fmt, ...) EV_CHECK_FMT(1,2);

diff --git a/log.c b/log.c
index c1f357c45234255c90ad75da27439e924bdfe67d..06f9efd000bd14d13733f3802e1cc13bb08b1761 100644 (file)
--- a/log.c
+++ b/log.c
@@ -59,6 +59,7 @@
 static void _warn_helper(int severity, const char *errstr, const char *fmt,
     va_list ap);
 static void event_log(int severity, const char *msg);
+static void event_exit(int errcode) EV_NORETURN;
 
 static event_fatal_cb fatal_fn = NULL;
 
@@ -71,9 +72,10 @@ event_set_fatal_callback(event_fatal_cb cb)
 static void
 event_exit(int errcode)
 {
-       if (fatal_fn)
+       if (fatal_fn) {
                fatal_fn(errcode);
-       else if (errcode == _EVENT_ERR_ABORT)
+               exit(errcode); /* should never be reached */
+       } else if (errcode == _EVENT_ERR_ABORT)
                abort();
        else
                exit(errcode);

diff --git a/test/regress_util.c b/test/regress_util.c
index 2e0d7722fa8fa1c852cd6f5dc336efad27d8ed37..1fd0797e0288e1169aa2feb30f657bdc69244993 100644 (file)
--- a/test/regress_util.c
+++ b/test/regress_util.c
@@ -423,15 +423,68 @@ logfn(int severity, const char *msg)
                logmsg = strdup(msg);
 }
 
-static int exited = 0;
-static int exitcode = 0;
+static int fatal_want_severity = 0;
+static const char *fatal_want_message = NULL;
 static void
-fatalfn(int c)
+fatalfn(int exitcode)
 {
-       exited = 1;
-       exitcode = c;
+       if (logsev != fatal_want_severity ||
+           !logmsg ||
+           strcmp(logmsg, fatal_want_message))
+               exit(0);
+       else
+               exit(exitcode);
 }
 
+#ifndef WIN32
+#define CAN_CHECK_ERR
+static void
+check_error_logging(void (*fn)(void), int wantexitcode,
+    int wantseverity, const char *wantmsg)
+{
+       pid_t pid;
+       int status = 0, exitcode;
+       fatal_want_severity = wantseverity;
+       fatal_want_message = wantmsg;
+       if ((pid = fork()) == 0) {
+               /* child process */
+               fn();
+               exit(0); /* should be unreachable. */
+       } else {
+               wait(&status);
+               exitcode = WEXITSTATUS(status);
+               tt_int_op(wantexitcode, ==, exitcode);
+       }
+end:
+       ;
+}
+
+static void
+errx_fn(void)
+{
+       event_errx(2, "Fatal error; too many kumquats (%d)", 5);
+}
+
+static void
+err_fn(void)
+{
+       errno = ENOENT;
+       event_err(5,"Couldn't open %s", "/very/bad/file");
+}
+
+static void
+sock_err_fn(void)
+{
+       evutil_socket_t fd = socket(AF_INET, SOCK_STREAM, 0);
+#ifdef WIN32
+       EVUTIL_SET_SOCKET_ERROR(WSAEWOULDBLOCK);
+#else
+       errno = EAGAIN;
+#endif
+       event_sock_err(20, fd, "Unhappy socket");
+}
+#endif
+
 static void
 test_evutil_log(void *ptr)
 {
@@ -441,7 +494,7 @@ test_evutil_log(void *ptr)
        event_set_log_callback(logfn);
        event_set_fatal_callback(fatalfn);
 #define RESET() do {                           \
-               logsev = exited = exitcode = 0; \
+               logsev = 0;     \
                if (logmsg) free(logmsg);       \
                logmsg = NULL;                  \
        } while (0)
@@ -451,19 +504,22 @@ test_evutil_log(void *ptr)
                tt_str_op(logmsg,==,msg);       \
        } while (0)
 
-       event_errx(2, "Fatal error; too many kumquats (%d)", 5);
-       LOGEQ(_EVENT_LOG_ERR, "Fatal error; too many kumquats (5)");
-       tt_int_op(exitcode,==,2);
+#ifdef CAN_CHECK_ERR
+       /* We need to disable these tests for now.  Previously, the logging
+        * module didn't enforce the requirement that a fatal callback
+        * actually exit.  Now, it exits no matter what, so if we wan to
+        * reinstate these tests, we'll need to fork for each one. */
+       check_error_logging(errx_fn, 2, _EVENT_LOG_ERR,
+           "Fatal error; too many kumquats (5)");
        RESET();
+#endif
 
        event_warnx("Far too many %s (%d)", "wombats", 99);
        LOGEQ(_EVENT_LOG_WARN, "Far too many wombats (99)");
-       tt_int_op(exited,==,0);
        RESET();
 
        event_msgx("Connecting lime to coconut");
        LOGEQ(_EVENT_LOG_MSG, "Connecting lime to coconut");
-       tt_int_op(exited,==,0);
        RESET();
 
        event_debug(("A millisecond passed!  We should log that!"));
@@ -481,16 +537,14 @@ test_evutil_log(void *ptr)
        evutil_snprintf(buf, sizeof(buf),
            "Couldn't open /bad/file: %s",strerror(ENOENT));
        LOGEQ(_EVENT_LOG_WARN,buf);
-       tt_int_op(exited, ==, 0);
        RESET();
 
-       errno = ENOENT;
-       event_err(5,"Couldn't open %s", "/very/bad/file");
+#ifdef CAN_CHECK_ERR
        evutil_snprintf(buf, sizeof(buf),
            "Couldn't open /very/bad/file: %s",strerror(ENOENT));
-       LOGEQ(_EVENT_LOG_ERR,buf);
-       tt_int_op(exitcode, ==, 5);
+       check_error_logging(err_fn, 5, _EVENT_LOG_ERR, buf);
        RESET();
+#endif
 
        /* Try with a socket errno. */
        fd = socket(AF_INET, SOCK_STREAM, 0);
@@ -506,18 +560,12 @@ test_evutil_log(void *ptr)
 #endif
        event_sock_warn(fd, "Unhappy socket");
        LOGEQ(_EVENT_LOG_WARN, buf);
-       tt_int_op(exited,==,0);
        RESET();
 
-#ifdef WIN32
-       EVUTIL_SET_SOCKET_ERROR(WSAEWOULDBLOCK);
-#else
-       errno = EAGAIN;
-#endif
-       event_sock_err(200, fd, "Unhappy socket");
-       LOGEQ(_EVENT_LOG_ERR, buf);
-       tt_int_op(exitcode,==,200);
+#ifdef CAN_CHECK_ERR
+       check_error_logging(sock_err_fn, 20, _EVENT_LOG_ERR, buf);
        RESET();
+#endif
 
 #undef RESET
 #undef LOGEQ