<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">2014-06-20 17:09 GMT+08:00 Michal Privoznik <<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>>: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On 17.06.2014 13:06, Taowei wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> 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. </blockquote> </div> s/rewriting/rewritten/<div class=""> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> 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) </blockquote> </div> We have ATTRIBUTE_UNUSED which can serve this purpose. Or ignore_value() depending on the use case.<div class=""> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> + +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) </blockquote> </div> so this would be: static int _initicalFWatch(vboxGlobalData *data ATTRIBUTE_UNUSED) { ... } even though the @data might be used later (depending on VBOX version).<div class=""> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> +{ +#if FWATCH_NEED_INITICAL == 0 </blockquote> </div> 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.<div class=""> </div></blockquote><div> </div><div><div>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.<div> I expect that others would know whether they need to initialize the fdWatch, so the flag and the fWatchNeedInitial is introduced.</div></div><div> </div><div>Do you mean just use VBOX_XPCOMC_VERSION and VBOX_API_VERSION to check directly?</div> </div><div> </div><div>I will modify the next codes with your advise. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> <div class=""> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> + /* 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; </blockquote> </div> s /unsigned .. :1/bool/ And what does Initical stands for anyway?<div class=""><div class="h5"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> +} 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", </blockquote> </div></div> Michal </blockquote></div> </div></div>