[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: 3/25: Remove use of macros from remote driver



"Daniel P. Berrange" <berrange redhat com> wrote:
> THis patch removes use of macros for accessing the privateDtaa
> on a connection because they obscure data access making it harder
> to visually validate correct thread locking
>
>  remote_internal.c |  272 ++++++++++++++++++++++--------------------------------
>  1 file changed, 114 insertions(+), 158 deletions(-)
>
> Daniel
>
> diff --git a/src/remote_internal.c b/src/remote_internal.c
> --- a/src/remote_internal.c
> +++ b/src/remote_internal.c
> @@ -86,14 +86,9 @@
>  #include "util.h"
>  #include "event.h"
>
> -/* Per-connection private data. */
> -#define MAGIC 999               /* private_data->magic if OK */
> -#define DEAD 998                /* private_data->magic if dead/closed */
> -
>  static int inside_daemon = 0;
>
>  struct private_data {
> -    int magic;                  /* Should be MAGIC or DEAD. */
>      int sock;                   /* Socket. */
>      int watch;                  /* File handle watch */
>      pid_t pid;                  /* PID of tunnel process */
> @@ -118,39 +113,6 @@ struct private_data {
>      /* Timer for flushing domainEvents queue */
>      int eventFlushTimer;
>  };
> -
> -#define GET_PRIVATE(conn,retcode)                                       \
> -    struct private_data *priv = (struct private_data *) (conn)->privateData; \
> -    if (!priv || priv->magic != MAGIC) {                                \
> -        error (conn, VIR_ERR_INVALID_ARG,                               \
> -               _("tried to use a closed or uninitialised handle"));     \
> -        return (retcode);                                               \
> -    }
> -
> -#define GET_NETWORK_PRIVATE(conn,retcode)                               \
> -    struct private_data *priv = (struct private_data *) (conn)->networkPrivateData; \
> -    if (!priv || priv->magic != MAGIC) {                                \
> -        error (conn, VIR_ERR_INVALID_ARG,                               \
> -               _("tried to use a closed or uninitialised handle"));     \
> -        return (retcode);                                               \
> -    }
> -
> -#define GET_STORAGE_PRIVATE(conn,retcode)                               \
> -    struct private_data *priv = (struct private_data *) (conn)->storagePrivateData; \
> -    if (!priv || priv->magic != MAGIC) {                                \
> -        error (conn, VIR_ERR_INVALID_ARG,                               \
> -               "tried to use a closed or uninitialised handle");        \
> -        return (retcode);                                               \
> -    }
> -
> -#define GET_DEVMON_PRIVATE(conn,retcode)                               \
> -    struct private_data *priv = (struct private_data *) (conn)->devMonPrivateData; \
> -    if (!priv || priv->magic != MAGIC) {                                \
> -        error (conn, VIR_ERR_INVALID_ARG,                               \
> -               _("tried to use a closed or uninitialised handle"));     \
> -        return (retcode);                                               \
> -    }

I offer to redo this change set, separating it into two parts:
  1) remove .magic and code from macros that manipulates it
  2) perform the substitutions: either automate that,
       or ensure that the generated code does not change.
Then there's almost no need for review.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]