Skip to content

Commit

Permalink
vlog: Make the most common module reference more direct.
Browse files Browse the repository at this point in the history
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@ovn.org>
Acked-by: Russell Bryant <russell@ovn.org>
  • Loading branch information
blp committed Feb 10, 2016
1 parent 3ca6cc7 commit 922fed0
Show file tree
Hide file tree
Showing 14 changed files with 59 additions and 59 deletions.
73 changes: 36 additions & 37 deletions include/openvswitch/vlog.h
Expand Up @@ -170,46 +170,45 @@ 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) \
/* This extra "extern" declaration makes sparse happy. */ \
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

/* 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.
\
/* Prevent duplicate module names, via linker error. \
* The extra "extern" declaration makes sparse happy. */ \
extern struct vlog_module *VLM_##MODULE; \
struct vlog_module *VLM_##MODULE = &this_module;

/* 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.
Expand All @@ -224,10 +223,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__)
Expand Down Expand Up @@ -267,22 +266,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)
Expand Down
4 changes: 2 additions & 2 deletions 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.
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/db-ctl-base.c
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions 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.
Expand Down Expand Up @@ -1583,7 +1583,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);
}

Expand Down Expand Up @@ -1616,7 +1616,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);
}

Expand Down Expand Up @@ -1696,7 +1696,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);
}
Expand Down
4 changes: 2 additions & 2 deletions 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.
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/ofp-prop.h
Expand Up @@ -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 */
4 changes: 2 additions & 2 deletions 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.
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions 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.
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion lib/vlog.c
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion ovn/utilities/ovn-nbctl.c
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions 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.
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion 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.
Expand Down
2 changes: 1 addition & 1 deletion utilities/ovs-vsctl.c
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions 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.
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 922fed0

Please sign in to comment.