[ovs-dev] [PATCH v2 4/6] vlog: Make the most common module reference more direct.
Ben Pfaff
blp at ovn.org
Wed Feb 3 21:50:09 UTC 2016
Most vlog calls are for the log module owned by the translation unit being
compiled, but this module was referenced indirectly through a pointer
variable. That seems silly, so this commit changes the code so that the
local vlog module is referred to directly, as &this_module.
We could get rid of the global variables for vlog modules entirely, but
I like getting linker errors when there's a duplicate module name.
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
include/openvswitch/vlog.h | 69 +++++++++++++++++++++++-----------------------
lib/bfd.c | 4 +--
lib/db-ctl-base.c | 2 +-
lib/dpif.c | 8 +++---
lib/jsonrpc.c | 4 +--
lib/ofp-prop.h | 2 +-
lib/stream-ssl.c | 4 +--
lib/vconn-stream.c | 4 +--
lib/vlog.c | 3 +-
ovn/utilities/ovn-nbctl.c | 2 +-
ovn/utilities/ovn-sbctl.c | 4 +--
tests/test-reconnect.c | 2 +-
utilities/ovs-vsctl.c | 2 +-
vtep/vtep-ctl.c | 4 +--
14 files changed, 57 insertions(+), 57 deletions(-)
diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
index ecf3786..bf30c49 100644
--- a/include/openvswitch/vlog.h
+++ b/include/openvswitch/vlog.h
@@ -170,45 +170,44 @@ void vlog_rate_limit(const struct vlog_module *, enum vlog_level,
struct vlog_rate_limit *, const char *, ...)
OVS_PRINTF_FORMAT (4, 5);
-/* Creates and initializes a global instance of a module named MODULE, and
- * defines a static variable named THIS_MODULE that points to it, for use with
- * the convenience macros below. */
+/* Defines a logging module whose name is MODULE, which should generally be
+ * roughly the name of the source file, and makes it the module used by the
+ * logging convenience macros defined below. */
#define VLOG_DEFINE_THIS_MODULE(MODULE) \
- extern struct vlog_module VLM_##MODULE; \
- struct vlog_module VLM_##MODULE = \
- { \
- OVS_LIST_INITIALIZER(&VLM_##MODULE.list), \
+ static struct vlog_module this_module = { \
+ OVS_LIST_INITIALIZER(&this_module.list), \
#MODULE, /* name */ \
{ VLL_INFO, VLL_INFO, VLL_INFO }, /* levels */ \
VLL_INFO, /* min_level */ \
true /* honor_rate_limits */ \
}; \
- OVS_CONSTRUCTOR(init_##MODULE) { \
- vlog_insert_module(&VLM_##MODULE.list); \
+ OVS_CONSTRUCTOR(init_this_module) { \
+ vlog_insert_module(&this_module.list); \
} \
- static struct vlog_module *const THIS_MODULE = &VLM_##MODULE
+ \
+ /* Prevent duplicate module names, via linker error. */ \
+ extern struct vlog_module *VLM_##MODULE; \
+ struct vlog_module *VLM_##MODULE = &this_module;
-/* Convenience macros. These assume that THIS_MODULE points to a "struct
- * vlog_module" for the current module, as set up by e.g. the
- * VLOG_DEFINE_THIS_MODULE macro above.
+/* Macros for the current module as set up by VLOG_DEFINE_THIS_MODULE.
+ * These are usually what you want to use.
*
* Guaranteed to preserve errno.
*/
-#define VLOG_FATAL(...) vlog_fatal(THIS_MODULE, __VA_ARGS__)
-#define VLOG_ABORT(...) vlog_abort(THIS_MODULE, __VA_ARGS__)
+#define VLOG_FATAL(...) vlog_fatal(&this_module, __VA_ARGS__)
+#define VLOG_ABORT(...) vlog_abort(&this_module, __VA_ARGS__)
#define VLOG_EMER(...) VLOG(VLL_EMER, __VA_ARGS__)
#define VLOG_ERR(...) VLOG(VLL_ERR, __VA_ARGS__)
#define VLOG_WARN(...) VLOG(VLL_WARN, __VA_ARGS__)
#define VLOG_INFO(...) VLOG(VLL_INFO, __VA_ARGS__)
#define VLOG_DBG(...) VLOG(VLL_DBG, __VA_ARGS__)
-/* More convenience macros, for testing whether a given level is enabled in
- * THIS_MODULE. When constructing a log message is expensive, this enables it
- * to be skipped. */
-#define VLOG_IS_ERR_ENABLED() vlog_is_enabled(THIS_MODULE, VLL_ERR)
-#define VLOG_IS_WARN_ENABLED() vlog_is_enabled(THIS_MODULE, VLL_WARN)
-#define VLOG_IS_INFO_ENABLED() vlog_is_enabled(THIS_MODULE, VLL_INFO)
-#define VLOG_IS_DBG_ENABLED() vlog_is_enabled(THIS_MODULE, VLL_DBG)
+/* More convenience macros, for testing whether a given level is enabled. When
+ * constructing a log message is expensive, this enables it to be skipped. */
+#define VLOG_IS_ERR_ENABLED() vlog_is_enabled(&this_module, VLL_ERR)
+#define VLOG_IS_WARN_ENABLED() vlog_is_enabled(&this_module, VLL_WARN)
+#define VLOG_IS_INFO_ENABLED() vlog_is_enabled(&this_module, VLL_INFO)
+#define VLOG_IS_DBG_ENABLED() vlog_is_enabled(&this_module, VLL_DBG)
/* Convenience macros for rate-limiting.
* Guaranteed to preserve errno.
@@ -223,10 +222,10 @@ void vlog_rate_limit(const struct vlog_module *, enum vlog_level,
#define VLOG_ERR_BUF(ERRP, ...) VLOG_ERRP(ERRP, VLL_ERR, __VA_ARGS__)
#define VLOG_WARN_BUF(ERRP, ...) VLOG_ERRP(ERRP, VLL_WARN, __VA_ARGS__)
-#define VLOG_DROP_ERR(RL) vlog_should_drop(THIS_MODULE, VLL_ERR, RL)
-#define VLOG_DROP_WARN(RL) vlog_should_drop(THIS_MODULE, VLL_WARN, RL)
-#define VLOG_DROP_INFO(RL) vlog_should_drop(THIS_MODULE, VLL_INFO, RL)
-#define VLOG_DROP_DBG(RL) vlog_should_drop(THIS_MODULE, VLL_DBG, RL)
+#define VLOG_DROP_ERR(RL) vlog_should_drop(&this_module, VLL_ERR, RL)
+#define VLOG_DROP_WARN(RL) vlog_should_drop(&this_module, VLL_WARN, RL)
+#define VLOG_DROP_INFO(RL) vlog_should_drop(&this_module, VLL_INFO, RL)
+#define VLOG_DROP_DBG(RL) vlog_should_drop(&this_module, VLL_DBG, RL)
/* Macros for logging at most once per execution. */
#define VLOG_ERR_ONCE(...) VLOG_ONCE(VLL_ERR, __VA_ARGS__)
@@ -266,22 +265,22 @@ void vlog_usage(void);
#define VLOG(LEVEL, ...) \
do { \
enum vlog_level level__ = LEVEL; \
- if (THIS_MODULE->min_level >= level__) { \
- vlog(THIS_MODULE, level__, __VA_ARGS__); \
+ if (this_module.min_level >= level__) { \
+ vlog(&this_module, level__, __VA_ARGS__); \
} \
} while (0)
-#define VLOG_RL(RL, LEVEL, ...) \
- do { \
- enum vlog_level level__ = LEVEL; \
- if (THIS_MODULE->min_level >= level__) { \
- vlog_rate_limit(THIS_MODULE, level__, RL, __VA_ARGS__); \
- } \
+#define VLOG_RL(RL, LEVEL, ...) \
+ do { \
+ enum vlog_level level__ = LEVEL; \
+ if (this_module.min_level >= level__) { \
+ vlog_rate_limit(&this_module, level__, RL, __VA_ARGS__); \
+ } \
} while (0)
#define VLOG_ONCE(LEVEL, ...) \
do { \
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; \
if (ovsthread_once_start(&once)) { \
- vlog(THIS_MODULE, LEVEL, __VA_ARGS__); \
+ vlog(&this_module, LEVEL, __VA_ARGS__); \
ovsthread_once_done(&once); \
} \
} while (0)
diff --git a/lib/bfd.c b/lib/bfd.c
index 66c99fb..8b5daff 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2013, 2014, 2015 Nicira, Inc.
+/* Copyright (c) 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1073,7 +1073,7 @@ log_msg(enum vlog_level level, const struct msg *p, const char *message,
{
struct ds ds = DS_EMPTY_INITIALIZER;
- if (vlog_should_drop(THIS_MODULE, level, &rl)) {
+ if (vlog_should_drop(&this_module, level, &rl)) {
return;
}
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index cf49ac0..e3a1de9 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1992,7 +1992,7 @@ ctl_fatal(const char *format, ...)
message = xvasprintf(format, args);
va_end(args);
- vlog_set_levels(&VLM_db_ctl_base, VLF_CONSOLE, VLL_OFF);
+ vlog_set_levels(&this_module, VLF_CONSOLE, VLL_OFF);
VLOG_ERR("%s", message);
ovs_error(0, "%s", message);
ctl_exit(EXIT_FAILURE);
diff --git a/lib/dpif.c b/lib/dpif.c
index 38e40ba..de4528c 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1584,7 +1584,7 @@ flow_message_log_level(int error)
static bool
should_log_flow_message(int error)
{
- return !vlog_should_drop(THIS_MODULE, flow_message_log_level(error),
+ return !vlog_should_drop(&this_module, flow_message_log_level(error),
error ? &error_rl : &dpmsg_rl);
}
@@ -1617,7 +1617,7 @@ log_flow_message(const struct dpif *dpif, int error, const char *operation,
ds_put_cstr(&ds, ", actions:");
format_odp_actions(&ds, actions, actions_len);
}
- vlog(THIS_MODULE, flow_message_log_level(error), "%s", ds_cstr(&ds));
+ vlog(&this_module, flow_message_log_level(error), "%s", ds_cstr(&ds));
ds_destroy(&ds);
}
@@ -1697,7 +1697,7 @@ log_execute_message(struct dpif *dpif, const struct dpif_execute *execute,
}
ds_put_format(&ds, " on packet %s", packet);
ds_put_format(&ds, " mtu %d", execute->mtu);
- vlog(THIS_MODULE, error ? VLL_WARN : VLL_DBG, "%s", ds_cstr(&ds));
+ vlog(&this_module, error ? VLL_WARN : VLL_DBG, "%s", ds_cstr(&ds));
ds_destroy(&ds);
free(packet);
}
diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index e6ee195..35428a6 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -343,7 +343,7 @@ jsonrpc_recv(struct jsonrpc *rpc, struct jsonrpc_msg **msgp)
const struct byteq *q = &rpc->input;
if (q->head <= q->size) {
stream_report_content(q->buffer, q->head, STREAM_JSONRPC,
- THIS_MODULE, rpc->name);
+ &this_module, rpc->name);
}
return rpc->status;
}
diff --git a/lib/ofp-prop.h b/lib/ofp-prop.h
index 2f07d07..921b6c1 100644
--- a/lib/ofp-prop.h
+++ b/lib/ofp-prop.h
@@ -129,6 +129,6 @@ void ofpprop_end(struct ofpbuf *, size_t start_ofs);
enum ofperr ofpprop_unknown(struct vlog_module *, bool loose, const char *msg,
uint64_t type);
#define OFPPROP_UNKNOWN(LOOSE, MSG, TYPE) \
- ofpprop_unknown(THIS_MODULE, LOOSE, MSG, TYPE)
+ ofpprop_unknown(&this_module, LOOSE, MSG, TYPE)
#endif /* ofp-prop.h */
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 0015fc3..2699633 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -455,7 +455,7 @@ ssl_connect(struct stream *stream)
: "SSL_accept"), retval, error, &unused);
shutdown(sslv->fd, SHUT_RDWR);
stream_report_content(sslv->head, sslv->n_head, STREAM_SSL,
- THIS_MODULE, stream_get_name(stream));
+ &this_module, stream_get_name(stream));
return EPROTO;
}
} else if (bootstrap_ca_cert) {
diff --git a/lib/vconn-stream.c b/lib/vconn-stream.c
index 23c0aae..2858462 100644
--- a/lib/vconn-stream.c
+++ b/lib/vconn-stream.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -104,7 +104,7 @@ vconn_stream_close(struct vconn *vconn)
if ((vconn->error == EPROTO || s->n_packets < 1) && s->rxbuf) {
stream_report_content(s->rxbuf->data, s->rxbuf->size, STREAM_OPENFLOW,
- THIS_MODULE, vconn_get_name(vconn));
+ &this_module, vconn_get_name(vconn));
}
stream_close(s->stream);
diff --git a/lib/vlog.c b/lib/vlog.c
index 3d0b87c..f514d65 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -209,7 +209,8 @@ vlog_get_destination_val(const char *name)
return i;
}
-void vlog_insert_module(struct ovs_list *vlog)
+void
+vlog_insert_module(struct ovs_list *vlog)
{
list_insert(&vlog_modules, vlog);
}
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 324a0a4..c0b6981 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -218,7 +218,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
break;
case OPT_NO_SYSLOG:
- vlog_set_levels(&VLM_nbctl, VLF_SYSLOG, VLL_WARN);
+ vlog_set_levels(&this_module, VLF_SYSLOG, VLL_WARN);
break;
case OPT_DRY_RUN:
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index b428a35..b9e3c10 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015 Nicira, Inc.
+ * Copyright (c) 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -231,7 +231,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
break;
case OPT_NO_SYSLOG:
- vlog_set_levels(&VLM_sbctl, VLF_SYSLOG, VLL_WARN);
+ vlog_set_levels(&this_module, VLF_SYSLOG, VLL_WARN);
break;
case OPT_DRY_RUN:
diff --git a/tests/test-reconnect.c b/tests/test-reconnect.c
index b70a7d5..76e43ea 100644
--- a/tests/test-reconnect.c
+++ b/tests/test-reconnect.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2014, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 00ac458..eeb711f 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -269,7 +269,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
break;
case OPT_NO_SYSLOG:
- vlog_set_levels(&VLM_vsctl, VLF_SYSLOG, VLL_WARN);
+ vlog_set_levels(&this_module, VLF_SYSLOG, VLL_WARN);
break;
case OPT_NO_WAIT:
diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index 2f13dc6..bf42f1c 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -223,7 +223,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
break;
case OPT_NO_SYSLOG:
- vlog_set_levels(&VLM_vtep_ctl, VLF_SYSLOG, VLL_WARN);
+ vlog_set_levels(&this_module, VLF_SYSLOG, VLL_WARN);
break;
case OPT_DRY_RUN:
--
2.1.3
More information about the dev
mailing list