[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

It's a trade-off, weighing loss of the magic-number check
against improved readability wrt threads and locking.
Sounds like it's the right way to go, ...

However, some of the transformations seem not
to have worked out properly:

> -#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);                                               \
> -    }


Here are a few of them:

> -    GET_NETWORK_PRIVATE (vol->conn, NULL);
> +    struct private_data *priv = vol->conn->storagePrivateData;
                                              ^^^^^^^
                                        should be ->networkPrivateData

> -    GET_STORAGE_PRIVATE (conn, -1);
> +    struct private_data *priv = conn->devMonPrivateData;
                                         ^^^^^^
                                         storagePrivateData

> -    GET_STORAGE_PRIVATE (conn, -1);
> +    struct private_data *priv = conn->devMonPrivateData;

> -    GET_STORAGE_PRIVATE (conn, -1);
> +    struct private_data *priv = conn->devMonPrivateData;

and a few more.


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