Makefile.am: add check that <assert.h> is not used from unexpected files.

In general, with a few specific exceptions, ovs_assert is now preferred
over assert, so this commit adds a check for that in the top-level
Makefile.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

diff --git a/Makefile.am b/Makefile.am
index 878a1a9..328b248 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -195,6 +195,19 @@ rate-limit-check:
         fi
 .PHONY: rate-limit-check
 
+# Check that assert.h is not used outside a whitelist of files.
+ALL_LOCAL += check-assert-h-usage
+check-assert-h-usage:
+       @if test -e $(srcdir)/.git && (git --version) >/dev/null 2>&1 && \
+           (cd $(srcdir) && git --no-pager grep -l -E '[<]assert.h[>]') | \
+           $(EGREP) -v '^lib/(sflow_receiver|vlog|worker).c$$|^tests/'; \
+         then \
+           echo "Files listed above unexpectedly #include <""assert.h"">."; \
+           echo "Please use ovs_assert (from util.h) instead of assert."; \
+           exit 1; \
+        fi
+.PHONY: check-assert-h-usage
+
 if HAVE_GROFF
 ALL_LOCAL += manpage-check
 manpage-check: $(man_MANS) $(dist_man_MANS) $(noinst_man_MANS)

diff --git a/configure.ac b/configure.ac
index 539d5e3..bcfb227 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -27,6 +27,7 @@ AC_PROG_CPP
 AC_PROG_RANLIB
 AC_PROG_MKDIR_P
 AC_PROG_FGREP
+AC_PROG_EGREP
 
 AC_ARG_VAR([PERL], [path to Perl interpreter])
 AC_PATH_PROG([PERL], perl, no)

diff --git a/lib/vlog.c b/lib/vlog.c
index 0bd9bd1..7867b07 100644 (file)
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,6 +28,7 @@
 #include <syslog.h>
 #include <time.h>
 #include <unistd.h>
+#include "coverage.h"
 #include "dirs.h"
 #include "dynamic-string.h"
 #include "ofpbuf.h"
@@ -40,6 +41,13 @@
 
 VLOG_DEFINE_THIS_MODULE(vlog);
 
+COVERAGE_DEFINE(vlog_recursive);
+
+/* ovs_assert() logs the assertion message, so using ovs_assert() in this
+ * source file could cause recursion. */
+#undef ovs_assert
+#define ovs_assert use_assert_instead_of_ovs_assert_in_this_module
+
 /* Name for each logging level. */
 static const char *level_names[VLL_N_LEVELS] = {
 #define VLOG_LEVEL(NAME, SYSLOG_LEVEL) #NAME,
@@ -480,6 +488,55 @@ vlog_unixctl_reopen(struct unixctl_conn *conn, int argc OVS_UNUSED,
     }
 }
 
+static void
+set_all_rate_limits(bool enable)
+{
+    struct vlog_module **mp;
+
+    for (mp = vlog_modules; mp < &vlog_modules[n_vlog_modules]; mp++) {
+        (*mp)->honor_rate_limits = enable;
+    }
+}
+
+static void
+set_rate_limits(struct unixctl_conn *conn, int argc,
+                const char *argv[], bool enable)
+{
+    if (argc > 1) {
+        int i;
+
+        for (i = 1; i < argc; i++) {
+            if (!strcasecmp(argv[i], "ANY")) {
+                set_all_rate_limits(enable);
+            } else {
+                struct vlog_module *module = vlog_module_from_name(argv[i]);
+                if (!module) {
+                    unixctl_command_reply_error(conn, "unknown module");
+                    return;
+                }
+                module->honor_rate_limits = enable;
+            }
+        }
+    } else {
+        set_all_rate_limits(enable);
+    }
+    unixctl_command_reply(conn, NULL);
+}
+
+static void
+vlog_enable_rate_limit(struct unixctl_conn *conn, int argc,
+                       const char *argv[], void *aux OVS_UNUSED)
+{
+    set_rate_limits(conn, argc, argv, true);
+}
+
+static void
+vlog_disable_rate_limit(struct unixctl_conn *conn, int argc,
+                       const char *argv[], void *aux OVS_UNUSED)
+{
+    set_rate_limits(conn, argc, argv, false);
+}
+
 /* Initializes the logging subsystem and registers its unixctl server
  * commands. */
 void
@@ -515,6 +572,10 @@ vlog_init(void)
         "vlog/set", "{spec | PATTERN:facility:pattern}",
         1, INT_MAX, vlog_unixctl_set, NULL);
     unixctl_command_register("vlog/list", "", 0, 0, vlog_unixctl_list, NULL);
+    unixctl_command_register("vlog/enable-rate-limit", "[module]...",
+                             0, INT_MAX, vlog_enable_rate_limit, NULL);
+    unixctl_command_register("vlog/disable-rate-limit", "[module]...",
+                             0, INT_MAX, vlog_disable_rate_limit, NULL);
     unixctl_command_register("vlog/reopen", "", 0, 0,
                              vlog_unixctl_reopen, NULL);
 }
@@ -543,12 +604,20 @@ vlog_get_levels(void)
     ds_put_format(&s, "                 -------    ------    ------\n");
 
     for (mp = vlog_modules; mp < &vlog_modules[n_vlog_modules]; mp++) {
-        line = xasprintf("%-16s  %4s       %4s       %4s\n",
-           vlog_get_module_name(*mp),
-           vlog_get_level_name(vlog_get_level(*mp, VLF_CONSOLE)),
-           vlog_get_level_name(vlog_get_level(*mp, VLF_SYSLOG)),
-           vlog_get_level_name(vlog_get_level(*mp, VLF_FILE)));
-        svec_add_nocopy(&lines, line);
+        struct ds line;
+
+        ds_init(&line);
+        ds_put_format(&line, "%-16s  %4s       %4s       %4s",
+                      vlog_get_module_name(*mp),
+                      vlog_get_level_name(vlog_get_level(*mp, VLF_CONSOLE)),
+                      vlog_get_level_name(vlog_get_level(*mp, VLF_SYSLOG)),
+                      vlog_get_level_name(vlog_get_level(*mp, VLF_FILE)));
+        if (!(*mp)->honor_rate_limits) {
+            ds_put_cstr(&line, "    (rate limiting disabled)");
+        }
+        ds_put_char(&line, '\n');
+
+        svec_add_nocopy(&lines, ds_steal_cstr(&line));
     }
 
     svec_sort(&lines);
@@ -826,6 +895,10 @@ bool
 vlog_should_drop(const struct vlog_module *module, enum vlog_level level,
                  struct vlog_rate_limit *rl)
 {
+    if (!module->honor_rate_limits) {
+        return false;
+    }
+
     if (!vlog_is_enabled(module, level)) {
         return true;
     }
@@ -887,13 +960,26 @@ static void
 vlog_write_file(struct ds *s)
 {
     if (worker_is_running()) {
-        worker_request(s->string, s->length,
-                       &log_fd, vlog_async_inited ? 0 : 1,
-                       vlog_async_write_request_cb, NULL, NULL);
-        vlog_async_inited = true;
-    } else {
-        ignore(write(log_fd, s->string, s->length));
+        static bool in_worker_request = false;
+        if (!in_worker_request) {
+            in_worker_request = true;
+
+            worker_request(s->string, s->length,
+                           &log_fd, vlog_async_inited ? 0 : 1,
+                           vlog_async_write_request_cb, NULL, NULL);
+            vlog_async_inited = true;
+
+            in_worker_request = false;
+            return;
+        } else {
+            /* We've been entered recursively.  This can happen if
+             * worker_request(), or a function that it calls, tries to log
+             * something.  We can't call worker_request() recursively, so fall
+             * back to writing the log file directly. */
+            COVERAGE_INC(vlog_recursive);
+        }
     }
+    ignore(write(log_fd, s->string, s->length));
 }
 
 static void

diff --git a/lib/worker.c b/lib/worker.c
index b6d268d..6ca05cd 100644 (file)
--- a/lib/worker.c
+++ b/lib/worker.c
@@ -37,6 +37,11 @@
 
 VLOG_DEFINE_THIS_MODULE(worker);
 
+/* ovs_assert() logs the assertion message and logging sometimes goes through a
+ * worker, so using ovs_assert() in this source file could cause recursion. */
+#undef ovs_assert
+#define ovs_assert use_assert_instead_of_ovs_assert_in_this_module
+
 /* Header for an RPC request. */
 struct worker_request {
     size_t request_len;              /* Length of the payload in bytes. */