[ovs-dev] [PATCH] Avoid C preprocessor trick where macro has the same name as a function.

Gurucharan Shetty shettyg at nicira.com
Mon Jul 29 17:52:52 UTC 2013


On Mon, Jul 29, 2013 at 9:57 AM, Ben Pfaff <blp at nicira.com> wrote:

> In C, one can do preprocessor tricks by making a macro expansion include
>
Usually we put a <area>: here. I am not sure whether you expect it on every
patch.


> the macro's own name.  We actually used this in the tree to automatically
> provide function arguments, e.g.:
>
>     #define f(x) f(x, __FILE__, __LINE__)
>     int f(int x, const char *file, int line);
>
> ...
>
>     f(1);    /* Expands to a call like f(1, __FILE__, __LINE__); */
>
> However it's somewhat confusing, so this commit stops using that trick.
>
> Reported-by: Ed Maste <emaste at freebsd.org>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/jsonrpc.c    |    2 +-
>  lib/latch.c      |   10 +++++++---
>  lib/latch.h      |    4 ++--
>  lib/ovs-thread.c |   16 ++++++++++++----
>  lib/ovs-thread.h |    8 ++++----
>  lib/poll-loop.c  |   37 ++++++++++++++++++-------------------
>  lib/poll-loop.h  |   20 +++++++++++---------
>  lib/timer.c      |   12 ++++++++----
>  lib/timer.h      |    6 +++---
>  9 files changed, 66 insertions(+), 49 deletions(-)
>
> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
> index 56b4cce..469fbae 100644
> --- a/lib/jsonrpc.c
> +++ b/lib/jsonrpc.c
> @@ -350,7 +350,7 @@ void
>  jsonrpc_recv_wait(struct jsonrpc *rpc)
>  {
>      if (rpc->status || rpc->received || !byteq_is_empty(&rpc->input)) {
> -        (poll_immediate_wake)(rpc->name);
> +        poll_immediate_wake_at(rpc->name);
>      } else {
>          stream_recv_wait(rpc->stream);
>      }
> diff --git a/lib/latch.c b/lib/latch.c
> index 9b13006..bf518b9 100644
> --- a/lib/latch.c
> +++ b/lib/latch.c
> @@ -75,9 +75,13 @@ latch_is_set(const struct latch *latch)
>      return pfd.revents & POLLIN;
>  }
>
> -/* Causes the next poll_block() to wake up when 'latch' is set. */
> +/* Causes the next poll_block() to wake up when 'latch' is set.
> + *
> + * ('where' is used in debug logging.  Commonly one would use
> latch_wait() to
> + * automatically provide the caller's source file and line number for
> + * 'where'.) */
>  void
> -(latch_wait)(const struct latch *latch, const char *where)
> +latch_wait_at(const struct latch *latch, const char *where)
>  {
> -    (poll_fd_wait)(latch->fds[0], POLLIN, where);
> +    poll_fd_wait_at(latch->fds[0], POLLIN, where);
>  }
> diff --git a/lib/latch.h b/lib/latch.h
> index 08f45b1..0b6e8a3 100644
> --- a/lib/latch.h
> +++ b/lib/latch.h
> @@ -36,7 +36,7 @@ bool latch_poll(struct latch *);
>  void latch_set(struct latch *);
>
>  bool latch_is_set(const struct latch *);
> -void latch_wait(const struct latch *, const char *where);
> -#define latch_wait(latch) latch_wait(latch, SOURCE_LOCATOR)
> +void latch_wait_at(const struct latch *, const char *where);
> +#define latch_wait(latch) latch_wait_at(latch, SOURCE_LOCATOR)
>
>  #endif /* latch.h */
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index d08751c..feff102 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -128,9 +128,13 @@ ovsthread_once_done(struct ovsthread_once *once)
>  }
>
>  /* Asserts that the process has not yet created any threads (beyond the
> initial
> - * thread).  */
> + * thread).
> + *
> + * ('where' is used in logging.  Commonly one would use
> + * assert_single_threaded() to automatically provide the caller's source
> file
> + * and line number for 'where'.) */
>  void
> -(assert_single_threaded)(const char *where)
> +assert_single_threaded_at(const char *where)
>  {
>      if (multithreaded) {
>          VLOG_FATAL("%s: attempted operation not allowed when
> multithreaded",
> @@ -140,9 +144,13 @@ void
>
>  /* Forks the current process (checking that this is allowed).  Aborts with
>   * VLOG_FATAL if fork() returns an error, and otherwise returns the value
> - * returned by fork().  */
> + * returned by fork().
> + *
> + * ('where' is used in logging.  Commonly one would use xfork() to
> + * automatically provide the caller's source file and line number for
> + * 'where'.) */
>  pid_t
> -(xfork)(const char *where)
> +xfork_at(const char *where)
>  {
>      pid_t pid;
>
> diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
> index 9c8023e..afb5ccd 100644
> --- a/lib/ovs-thread.h
> +++ b/lib/ovs-thread.h
> @@ -368,11 +368,11 @@ ovsthread_once_start(struct ovsthread_once *once)
>      ((ONCE)->done ? false : ({ OVS_ACQUIRE(ONCE); true; }))
>  #endif
>
> -void assert_single_threaded(const char *where);
> -#define assert_single_threaded() assert_single_threaded(SOURCE_LOCATOR)
> +void assert_single_threaded_at(const char *where);
> +#define assert_single_threaded() assert_single_threaded_at(SOURCE_LOCATOR)
>
> -pid_t xfork(const char *where);
> -#define xfork() xfork(SOURCE_LOCATOR)
> +pid_t xfork_at(const char *where);
> +#define xfork() xfork_at(SOURCE_LOCATOR)
>
>  void forbid_forking(const char *reason);
>  bool may_fork(void);
> diff --git a/lib/poll-loop.c b/lib/poll-loop.c
> index 5f4b16c..0e11c7e 100644
> --- a/lib/poll-loop.c
> +++ b/lib/poll-loop.c
> @@ -29,11 +29,6 @@
>  #include "timeval.h"
>  #include "vlog.h"
>
> -#undef poll_fd_wait
> -#undef poll_timer_wait
> -#undef poll_timer_wait_until
> -#undef poll_immediate_wake
> -
>  VLOG_DEFINE_THIS_MODULE(poll_loop);
>
>  COVERAGE_DEFINE(poll_fd_wait);
> @@ -73,10 +68,11 @@ static struct poll_waiter *new_waiter(int fd, short
> int events,
>   * is affected.  The event will need to be re-registered after
> poll_block() is
>   * called if it is to persist.
>   *
> - * Ordinarily the 'where' argument is supplied automatically; see
> poll-loop.h
> - * for more information. */
> + * ('where' is used in debug logging.  Commonly one would use
> poll_fd_wait() to
> + * automatically provide the caller's source file and line number for
> + * 'where'.) */
>  struct poll_waiter *
> -poll_fd_wait(int fd, short int events, const char *where)
> +poll_fd_wait_at(int fd, short int events, const char *where)
>  {
>      COVERAGE_INC(poll_fd_wait);
>      return new_waiter(fd, events, where);
> @@ -90,10 +86,11 @@ poll_fd_wait(int fd, short int events, const char
> *where)
>   * is affected.  The timer will need to be re-registered after
> poll_block() is
>   * called if it is to persist.
>   *
> - * Ordinarily the 'where' argument is supplied automatically; see
> poll-loop.h
> - * for more information. */
> + * ('where' is used in debug logging.  Commonly one would use
> poll_timer_wait()
> + * to automatically provide the caller's source file and line number for
> + * 'where'.) */
>  void
> -poll_timer_wait(long long int msec, const char *where)
> +poll_timer_wait_at(long long int msec, const char *where)
>  {
>      long long int now = time_msec();
>      long long int when;
> @@ -109,7 +106,7 @@ poll_timer_wait(long long int msec, const char *where)
>          when = LLONG_MAX;
>      }
>
> -    poll_timer_wait_until(when, where);
> +    poll_timer_wait_until_at(when, where);
>  }
>
>  /* Causes the following call to poll_block() to wake up when the current
> time,
> @@ -121,10 +118,11 @@ poll_timer_wait(long long int msec, const char
> *where)
>   * is affected.  The timer will need to be re-registered after
> poll_block() is
>   * called if it is to persist.
>   *
> - * Ordinarily the 'where' argument is supplied automatically; see
> poll-loop.h
> - * for more information. */
> + * ('where' is used in debug logging.  Commonly one would use
> + * poll_timer_wait_until() to automatically provide the caller's source
> file
> + * and line number for 'where'.) */
>  void
> -poll_timer_wait_until(long long int when, const char *where)
> +poll_timer_wait_until_at(long long int when, const char *where)
>  {
>      if (when < timeout_when) {
>          timeout_when = when;
> @@ -135,12 +133,13 @@ poll_timer_wait_until(long long int when, const char
> *where)
>  /* Causes the following call to poll_block() to wake up immediately,
> without
>   * blocking.
>   *
> - * Ordinarily the 'where' argument is supplied automatically; see
> poll-loop.h
> - * for more information. */
> + * ('where' is used in debug logging.  Commonly one would use
> + * poll_immediate_wake() to automatically provide the caller's source
> file and
> + * line number for 'where'.) */
>  void
> -poll_immediate_wake(const char *where)
> +poll_immediate_wake_at(const char *where)
>  {
> -    poll_timer_wait(0, where);
> +    poll_timer_wait_at(0, where);
>  }
>
>  /* Logs, if appropriate, that the poll loop was awakened by an event
> diff --git a/lib/poll-loop.h b/lib/poll-loop.h
> index 4c488ee..0b3f687 100644
> --- a/lib/poll-loop.h
> +++ b/lib/poll-loop.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 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.
> @@ -47,17 +47,19 @@ struct poll_waiter;
>   *      (poll_fd_wait)(fd, events, where);
>
Shouldn't we change the above example to poll_fd_wait_at?

>   * See timer_wait() for an example.
>   */
> -struct poll_waiter *poll_fd_wait(int fd, short int events, const char
> *where);
> -#define poll_fd_wait(fd, events) poll_fd_wait(fd, events, SOURCE_LOCATOR)
> +struct poll_waiter *poll_fd_wait_at(int fd, short int events,
> +                                    const char *where);
> +#define poll_fd_wait(fd, events) poll_fd_wait_at(fd, events,
> SOURCE_LOCATOR)
>
> -void poll_timer_wait(long long int msec, const char *where);
> -#define poll_timer_wait(msec) poll_timer_wait(msec, SOURCE_LOCATOR)
> +void poll_timer_wait_at(long long int msec, const char *where);
> +#define poll_timer_wait(msec) poll_timer_wait_at(msec, SOURCE_LOCATOR)
>
> -void poll_timer_wait_until(long long int msec, const char *where);
> -#define poll_timer_wait_until(msec) poll_timer_wait_until(msec,
> SOURCE_LOCATOR)
> +void poll_timer_wait_until_at(long long int msec, const char *where);
> +#define poll_timer_wait_until(msec)             \
> +    poll_timer_wait_until_at(msec, SOURCE_LOCATOR)
>
> -void poll_immediate_wake(const char *where);
> -#define poll_immediate_wake() poll_immediate_wake(SOURCE_LOCATOR)
> +void poll_immediate_wake_at(const char *where);
> +#define poll_immediate_wake() poll_immediate_wake_at(SOURCE_LOCATOR)
>
>  /* Wait until an event occurs. */
>  void poll_block(void);
> diff --git a/lib/timer.c b/lib/timer.c
> index e767db6..e90355b 100644
> --- a/lib/timer.c
> +++ b/lib/timer.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2011 Nicira, Inc.
> + * Copyright (c) 2011, 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.
> @@ -32,11 +32,15 @@ timer_msecs_until_expired(const struct timer *timer)
>      }
>  }
>
> -/* Causes poll_block() to wake when 'timer' expires. */
> +/* Causes poll_block() to wake when 'timer' expires.
> + *
> + * ('where' is used in debug logging.  Commonly one would use
> timer_wait() to
> + * automatically provide the caller's source file and line number for
> + * 'where'.) */
>  void
> -(timer_wait)(const struct timer *timer, const char *where)
> +timer_wait_at(const struct timer *timer, const char *where)
>  {
>      if (timer->t < LLONG_MAX) {
> -        (poll_timer_wait_until)(timer->t, where);
> +        poll_timer_wait_until_at(timer->t, where);
>      }
>  }
> diff --git a/lib/timer.h b/lib/timer.h
> index e9650ad..9afe3b7 100644
> --- a/lib/timer.h
> +++ b/lib/timer.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2011 Nicira, Inc.
> + * Copyright (c) 2011, 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.
> @@ -27,8 +27,8 @@ struct timer {
>  };
>
>  long long int timer_msecs_until_expired(const struct timer *);
> -void timer_wait(const struct timer *, const char *where);
> -#define timer_wait(timer) timer_wait(timer, SOURCE_LOCATOR)
> +void timer_wait_at(const struct timer *, const char *where);
> +#define timer_wait(timer) timer_wait_at(timer, SOURCE_LOCATOR)
>
>  /* Causes 'timer' to expire when 'duration' milliseconds have passed.
>
Looks good to me.
Thanks,
Guru

>   *
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130729/c2e3c6ff/attachment-0003.html>


More information about the dev mailing list