4:58 a.m.
2014-06-20 17:09 GMT+08:00 Michal Privoznik <mprivozn(a)redhat.com>:
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.
The fWatchNeedInitial is a flag that indicates whether we need to initial
fdWatch and vboxQueue in vboxGlobalData. VBox2.2 has no event call back, so
we don't need to call it.
I expect that others would know whether they need to initialize the
fdWatch, so the flag and the fWatchNeedInitial is introduced.
Do you mean just use VBOX_XPCOMC_VERSION and VBOX_API_VERSION to check
directly?
I will modify the next codes with your advise.
+ /* 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