[libvirt] [PATCH] outline of writing vbox driver

[Michal Privoznik]

On 17.06.2014 13:06, Taowei wrote:
> Define the vboxUniformedAPI struct to handle version conflicts.
> The vboxInitialize is rewrited with the new mechanism. Other
> functions would be rewriting in the same way.

s/rewriting/rewritten/

>
> Here, I still use template to generate functions in vboxUniformedAPI.
> Though, these functions may change between different versions, but
> not for every version. Using template could decrease the duplicated code.
>
> For every new feature added by vbox, a flag would indicate whether this
> feature is supported in current version. Calling for an unsupported
> feature would lead to a VIR_WARN.
>
> ---
>   src/vbox/vbox_tmpl.c |   84 ++++++++++++++++++++++++++++++++++++--------------
>   1 file changed, 61 insertions(+), 23 deletions(-)
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 1ed2729..4625805 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -826,6 +826,65 @@ static PRUnichar *PRUnicharFromInt(int n) {
>
>   #endif /* !(VBOX_API_VERSION == 2002000) */
>
> +/* Begin of vboxUniformedAPI */
> +
> +#define UNUSED(expr) do { (void)(expr); } while (0)

We have ATTRIBUTE_UNUSED which can serve this purpose. Or ignore_value() 
depending on the use case.

> +
> +static void _pfnComInitialize(vboxGlobalData *data)
> +{
> +#if VBOX_XPCOMC_VERSION == 0x00010000U
> +    data->pFuncs->pfnComInitialize(&data->vboxObj, &data->vboxSession);
> +#else  /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */
> +    data->pFuncs->pfnComInitialize(IVIRTUALBOX_IID_STR, &data->vboxObj, ISESSION_IID_STR, &data->vboxSession);
> +#endif /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */
> +}
> +
> +#if (VBOX_XPCOMC_VERSION == 0x00010000U) || (VBOX_API_VERSION == 2002000)
> +    #define FWATCH_NEED_INITICAL 0
> +#else /* (VBOX_XPCOMC_VERSION != 0x00010000U && VBOX_API_VERSION != 2002000) */
> +    #define FWATCH_NEED_INITICAL 1
> +#endif /* (VBOX_XPCOMC_VERSION != 0x00010000U && VBOX_API_VERSION != 2002000) */
> +
> +static int _initicalFWatch(vboxGlobalData *data)

so this would be:
static int
_initicalFWatch(vboxGlobalData *data ATTRIBUTE_UNUSED)
{ ... }

even though the @data might be used later (depending on VBOX version).

> +{
> +#if FWATCH_NEED_INITICAL == 0

What's the reason for not having:

#ifdef VBOX_XPCOMC_VERSION .. || VBOX_API_VERSION ..

#else

#endinf

I don't think I see why you need to invent this new preprocessor symbol.

> +    /* No event queue functionality in 2.2.* as of now */
> +    UNUSED(data);
> +    VIR_WARN("There is no fWatch initical in current version");
> +#else /* FWATCH_NEED_INITICAL != 0 */
> +    /* Initial the fWatch needed for Event Callbacks */
> +    data->fdWatch = -1;
> +    data->pFuncs->pfnGetEventQueue(&data->vboxQueue);
> +    if (data->vboxQueue == NULL) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("nsIEventQueue object is null"));
> +        return -1;
> +    }
> +#endif /* FWATCH_NEED_INITICAL != 0 */
> +    return 0;
> +}
> +
> +typedef struct {
> +    /* vbox API version */
> +    uint32_t uVersion;
> +    /* vbox APIs */
> +    void  (*pfnComInitialize)(vboxGlobalData *data);
> +    int (*initicalFWatch)(vboxGlobalData *data);
> +    /* vbox API features */
> +    unsigned fWatchNeedInitical : 1;

s /unsigned .. :1/bool/

And what does Initical stands for anyway?

> +} vboxUniformedAPI;
> +
> +static vboxUniformedAPI vboxAPI = {
> +    .uVersion = VBOX_API_VERSION,
> +    .pfnComInitialize = _pfnComInitialize,
> +    .initicalFWatch = _initicalFWatch,
> +    .fWatchNeedInitical = FWATCH_NEED_INITICAL,
> +};
> +
> +static vboxUniformedAPI *pVboxAPI = &vboxAPI;
> +
> +/* End of vboxUniformedAPI and Begin of common codes */
> +
>   static PRUnichar *
>   vboxSocketFormatAddrUtf16(vboxGlobalData *data, virSocketAddrPtr addr)
>   {
> @@ -923,31 +982,10 @@ vboxInitialize(vboxGlobalData *data)
>       if (data->pFuncs == NULL)
>           goto cleanup;
>
> -#if VBOX_XPCOMC_VERSION == 0x00010000U
> -    data->pFuncs->pfnComInitialize(&data->vboxObj, &data->vboxSession);
> -#else  /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */
> -    data->pFuncs->pfnComInitialize(IVIRTUALBOX_IID_STR, &data->vboxObj,
> -                               ISESSION_IID_STR, &data->vboxSession);
> -
> -# if VBOX_API_VERSION == 2002000
> -
> -    /* No event queue functionality in 2.2.* as of now */
> -
> -# else  /* !(VBOX_API_VERSION == 2002000) */
> +    pVboxAPI->pfnComInitialize(data);
>
> -    /* Initial the fWatch needed for Event Callbacks */
> -    data->fdWatch = -1;
> -
> -    data->pFuncs->pfnGetEventQueue(&data->vboxQueue);
> -
> -    if (data->vboxQueue == NULL) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("nsIEventQueue object is null"));
> +    if (pVboxAPI->fWatchNeedInitical && pVboxAPI->initicalFWatch(data) != 0)
>           goto cleanup;
> -    }
> -
> -# endif /* !(VBOX_API_VERSION == 2002000) */
> -#endif /* !(VBOX_XPCOMC_VERSION == 0x00010000U) */
>
>       if (data->vboxObj == NULL) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>

Michal