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

…les.

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 <[email protected]>
Acked-by: Ethan Jackson <[email protected]>

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)

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)

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

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. */