Re: [PATCH 1/3] vz: simplify getting strings from vzsdk

[Maxim Nestratov <mnestratov@xxxxxxxxxxxxx>]

06.04.2016 14:42, Nikolay Shirokovskiy пишет:
> SDK does not allocate memory when getting strings thus we
> need to call every function that returns string twice.
> First to obtain string length, second to obtain string
> itself. It is tedious. Let's move this logic into macro
> that generates wrapper. Luckily all functions we need
> to wrap have 3 parameters: some handle, pointer to
> string and string length.
>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
> ---
>   src/vz/vz_sdk.c | 167 +++++++++++++++++++-------------------------------------
>   1 file changed, 56 insertions(+), 111 deletions(-)
>
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 8691887..0d853d0 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -201,6 +201,44 @@ waitJobHelper(PRL_HANDLE job, unsigned int timeout,
>       waitJobHelper(job, JOB_INFINIT_WAIT_TIMEOUT, __FILE__,  \
>                     __FUNCTION__, __LINE__)
>   
> +#define PRLSDK_GENERATE_STRING_GETTER(FUN)          \
> +static char*                                        \
> +FUN ## _Alloc(PRL_HANDLE handle)                    \
> +{                                                   \
> +    PRL_RESULT pret;                                \
> +    PRL_UINT32 buflen = 0;                          \
> +    char *str = NULL;                               \
> +                                                    \
> +    pret = FUN(handle, NULL, &buflen);              \
> +    prlsdkCheckRetGoto(pret, error);                \
> +                                                    \
> +    if (VIR_ALLOC_N(str, buflen) < 0)               \
> +        goto error;                                 \
> +                                                    \
> +    pret = FUN(handle, str, &buflen);               \
> +    prlsdkCheckRetGoto(pret, error);                \
> +                                                    \
> +    return str;                                     \
> +                                                    \
> + error:                                             \
> +    VIR_FREE(str);                                  \
> +    return NULL;                                    \
> +}

I'd prefer to see a real function, not a macro with a pointer to 
necessary Prlxxx  function as a parameter
like:

static char * prlsdkGetStringParam(PRL_HANDLE sdkdom, 
prlsdkGetStringFunc func)

...
    pret = func(sdkdom, src, &buflen);
...

> +
> +PRLSDK_GENERATE_STRING_GETTER(PrlResult_GetParamAsString)
> +
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmCfg_GetName)
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmCfg_GetVNCHostName)
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmCfg_GetCpuMask)
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmCfg_GetHomePath)
> +
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmDev_GetFriendlyName)
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmDev_GetImagePath)
> +
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmDevHd_GetMountPoint)
> +
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmDevNet_GetHostInterfaceName)
> +PRLSDK_GENERATE_STRING_GETTER(PrlVmDevNet_GetVirtualNetworkId)
>   
>   int
>   prlsdkInit(void)
> @@ -346,19 +384,8 @@ prlsdkGetDomainIds(PRL_HANDLE sdkdom,
>       PRL_UINT32 len;
>       PRL_RESULT pret;
>   
> -    if (name) {
> -        len = 0;
> -        *name = NULL;
> -        /* get name length */
> -        pret = PrlVmCfg_GetName(sdkdom, NULL, &len);
> -        prlsdkCheckRetGoto(pret, error);
> -
> -        if (VIR_ALLOC_N(*name, len) < 0)
> -            goto error;
> -
> -        pret = PrlVmCfg_GetName(sdkdom, *name, &len);
> -        prlsdkCheckRetGoto(pret, error);
> -    }
> +    if (name && !(*name = PrlVmCfg_GetName_Alloc(sdkdom)))
> +        goto error;
>   
>       if (uuid) {
>           len = sizeof(uuidstr);
> @@ -465,7 +492,6 @@ prlsdkGetDiskInfo(vzConnPtr privconn,
>                     bool isCt)
>   {
>       char *buf = NULL;
> -    PRL_UINT32 buflen = 0;
>       PRL_RESULT pret;
>       PRL_UINT32 emulatedType;
>       PRL_UINT32 ifType;
> @@ -497,15 +523,9 @@ prlsdkGetDiskInfo(vzConnPtr privconn,
>           disk->device = VIR_DOMAIN_DISK_DEVICE_DISK;
>       }
>   
> -    pret = PrlVmDev_GetFriendlyName(prldisk, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
> -    if (VIR_ALLOC_N(buf, buflen) < 0)
> +    if (!(buf = PrlVmDev_GetFriendlyName_Alloc(prldisk)))
>           goto cleanup;
>   
> -    pret = PrlVmDev_GetFriendlyName(prldisk, buf, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
>       if (virDomainDiskSetSource(disk, buf) < 0)
>           goto cleanup;
>   
> @@ -567,8 +587,6 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk,
>                   virDomainFSDefPtr fs)
>   {
>       char *buf = NULL;
> -    PRL_UINT32 buflen = 0;
> -    PRL_RESULT pret;
>       int ret = -1;
>   
>       fs->type = VIR_DOMAIN_FS_TYPE_FILE;
> @@ -580,27 +598,15 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk,
>       fs->readonly = false;
>       fs->symlinksResolved = false;
>   
> -    pret = PrlVmDev_GetImagePath(prldisk, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
> -    if (VIR_ALLOC_N(buf, buflen) < 0)
> +    if (!(buf = PrlVmDev_GetImagePath_Alloc(prldisk)))
>           goto cleanup;
>   
> -    pret = PrlVmDev_GetImagePath(prldisk, buf, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
>       fs->src = buf;
>       buf = NULL;
>   
> -    pret = PrlVmDevHd_GetMountPoint(prldisk, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
> -    if (VIR_ALLOC_N(buf, buflen) < 0)
> +    if (!(buf = PrlVmDevHd_GetMountPoint_Alloc(prldisk)))
>           goto cleanup;
>   
> -    pret = PrlVmDevHd_GetMountPoint(prldisk, buf, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
>       fs->dst = buf;
>       buf = NULL;
>   
> @@ -726,15 +732,9 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt)
>   
>       /* use device name, shown by prlctl as target device
>        * for identifying network adapter in virDomainDefineXML */
> -    pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
> -    if (VIR_ALLOC_N(net->ifname, buflen) < 0)
> +    if (!(net->ifname = PrlVmDevNet_GetHostInterfaceName_Alloc(netAdapter)))
>           goto cleanup;
>   
> -    pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net->ifname, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
>       pret = PrlVmDev_GetIndex(netAdapter, &netAdapterIndex);
>       prlsdkCheckRetGoto(pret, cleanup);
>   
> @@ -765,17 +765,10 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt)
>                          PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
>               goto cleanup;
>       } else {
> -        pret = PrlVmDevNet_GetVirtualNetworkId(netAdapter, NULL, &buflen);
> -        prlsdkCheckRetGoto(pret, cleanup);
> -
> -        if (VIR_ALLOC_N(net->data.network.name, buflen) < 0)
> +        if (!(net->data.network.name =
> +              PrlVmDevNet_GetVirtualNetworkId_Alloc(netAdapter)))
>               goto cleanup;
>   
> -        pret = PrlVmDevNet_GetVirtualNetworkId(netAdapter,
> -                                               net->data.network.name,
> -                                               &buflen);
> -        prlsdkCheckRetGoto(pret, cleanup);
> -
>           /*
>            * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters
>            * except those whose Virtual Network Id differ from Parallels
> @@ -868,7 +861,6 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDefPtr chr)
>       PRL_UINT32 serialPortIndex;
>       PRL_UINT32 emulatedType;
>       char *friendlyName = NULL;
> -    PRL_UINT32 buflen;
>   
>       chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
>       chr->targetTypeAttr = false;
> @@ -879,15 +871,9 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort, virDomainChrDefPtr chr)
>       pret = PrlVmDev_GetEmulatedType(serialPort, &emulatedType);
>       prlsdkCheckRetGoto(pret, error);
>   
> -    pret = PrlVmDev_GetFriendlyName(serialPort, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, error);
> -
> -    if (VIR_ALLOC_N(friendlyName, buflen) < 0)
> +    if (!(friendlyName = PrlVmDev_GetFriendlyName_Alloc(serialPort)))
>           goto error;
>   
> -    pret = PrlVmDev_GetFriendlyName(serialPort, friendlyName, &buflen);
> -    prlsdkCheckRetGoto(pret, error);
> -
>       switch (emulatedType) {
>       case PDT_USE_OUTPUT_FILE:
>           chr->source.type = VIR_DOMAIN_CHR_TYPE_FILE;
> @@ -987,7 +973,6 @@ prlsdkAddVNCInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
>       virDomainGraphicsDefPtr gr = NULL;
>       PRL_VM_REMOTE_DISPLAY_MODE vncMode;
>       PRL_UINT32 port;
> -    PRL_UINT32 buflen = 0;
>       PRL_RESULT pret;
>   
>       pret = PrlVmCfg_GetVNCMode(sdkdom, &vncMode);
> @@ -1016,15 +1001,9 @@ prlsdkAddVNCInfo(PRL_HANDLE sdkdom, virDomainDefPtr def)
>   
>       gr->nListens = 1;
>   
> -    pret = PrlVmCfg_GetVNCHostName(sdkdom, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, error);
> -
> -    if (VIR_ALLOC_N(gr->listens[0].address, buflen) < 0)
> +    if (!(gr->listens[0].address = PrlVmCfg_GetVNCHostName_Alloc(sdkdom)))
>           goto error;
>   
> -    pret = PrlVmCfg_GetVNCHostName(sdkdom, gr->listens[0].address, &buflen);
> -    prlsdkCheckRetGoto(pret, error);
> -
>       gr->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS;
>   
>       if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, gr) < 0)
> @@ -1139,7 +1118,6 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom,
>                        virDomainDefPtr def)
>   {
>       char *buf;
> -    PRL_UINT32 buflen = 0;
>       int hostcpus;
>       PRL_UINT32 cpuCount;
>       PRL_RESULT pret;
> @@ -1161,14 +1139,9 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom,
>       if (virDomainDefSetVcpus(def, cpuCount) < 0)
>           goto cleanup;
>   
> -    pret = PrlVmCfg_GetCpuMask(sdkdom, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
> -    if (VIR_ALLOC_N(buf, buflen) < 0)
> +    if (!(buf = PrlVmCfg_GetCpuMask_Alloc(sdkdom)))
>           goto cleanup;
>   
> -    pret = PrlVmCfg_GetCpuMask(sdkdom, buf, &buflen);
> -
>       if (strlen(buf) == 0) {
>           if (!(def->cpumask = virBitmapNew(hostcpus)))
>               goto cleanup;
> @@ -1273,7 +1246,6 @@ prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom)
>       VIRTUAL_MACHINE_STATE domainState;
>       char *home = NULL;
>   
> -    PRL_UINT32 buflen = 0;
>       PRL_RESULT pret;
>       PRL_UINT32 ram;
>       PRL_UINT32 envId;
> @@ -1326,16 +1298,9 @@ prlsdkLoadDomain(vzConnPtr privconn, virDomainObjPtr dom)
>       pret = PrlVmCfg_GetEnvId(sdkdom, &envId);
>       prlsdkCheckRetGoto(pret, error);
>   
> -    buflen = 0;
> -    pret = PrlVmCfg_GetHomePath(sdkdom, NULL, &buflen);
> -    prlsdkCheckRetGoto(pret, error);
> -
> -    if (VIR_ALLOC_N(home, buflen) < 0)
> +    if (!(home = PrlVmCfg_GetHomePath_Alloc(sdkdom)))
>           goto error;
>   
> -    pret = PrlVmCfg_GetHomePath(sdkdom, home, &buflen);
> -    prlsdkCheckRetGoto(pret, error);
> -
>       /* For VMs home is actually /directory/config.pvs */
>       if (!IS_CT(def)) {
>           /* Get rid of /config.pvs in path string */
> @@ -3190,7 +3155,6 @@ prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk)
>   {
>       int idx = -1;
>       char *buf = NULL;
> -    PRL_UINT32 buflen = 0;
>       PRL_RESULT pret;
>       PRL_UINT32 hddCount;
>       PRL_UINT32 i;
> @@ -3204,16 +3168,9 @@ prlsdkGetDiskIndex(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk)
>           pret = PrlVmCfg_GetHardDisk(sdkdom, i, &hdd);
>           prlsdkCheckRetGoto(pret, cleanup);
>   
> -        buflen = 0;
> -        pret = PrlVmDev_GetFriendlyName(hdd, 0, &buflen);
> -        prlsdkCheckRetGoto(pret, cleanup);
> -
> -        if (VIR_ALLOC_N(buf, buflen) < 0)
> +        if (!(buf = PrlVmDev_GetFriendlyName_Alloc(hdd)))
>               goto cleanup;
>   
> -        pret = PrlVmDev_GetFriendlyName(hdd, buf, &buflen);
> -        prlsdkCheckRetGoto(pret, cleanup);
> -
>           if (STRNEQ(disk->src->path, buf)) {
>   
>               PrlHandle_Free(hdd);
> @@ -3613,7 +3570,7 @@ prlsdkDomainHasSnapshots(PRL_HANDLE sdkdom, int* found)
>       PRL_HANDLE job;
>       PRL_HANDLE result;
>       char *snapshotxml = NULL;
> -    unsigned int len, paramsCount;
> +    unsigned int paramsCount;
>       xmlDocPtr xml = NULL;
>       xmlXPathContextPtr ctxt = NULL;
>   
> @@ -3630,16 +3587,10 @@ prlsdkDomainHasSnapshots(PRL_HANDLE sdkdom, int* found)
>       if (!paramsCount)
>           goto cleanup;
>   
> -    pret = PrlResult_GetParamAsString(result, 0, &len);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
> -    if (VIR_ALLOC_N(snapshotxml, len+1) < 0)
> +    if (!(snapshotxml = PrlResult_GetParamAsString_Alloc(result)))
>           goto cleanup;
>   
> -    pret = PrlResult_GetParamAsString(result, snapshotxml, &len);
> -    prlsdkCheckRetGoto(pret, cleanup);
> -
> -    if (len <= 1) {
> +    if (*snapshotxml == '\0') {
>           /* The document is empty that means no snapshots */
>           *found = 0;
>           ret = 0;
> @@ -3848,12 +3799,12 @@ prlsdkGetBlockStats(virDomainObjPtr dom, virDomainDiskDefPtr disk, virDomainBloc
>       return ret;
>   }
>   
> +
>   static PRL_HANDLE
>   prlsdkFindNetByPath(virDomainObjPtr dom, const char *path)
>   {
>       PRL_UINT32 count = 0;
>       vzDomObjPtr privdom = dom->privateData;
> -    PRL_UINT32 buflen = 0;
>       PRL_RESULT pret;
>       size_t i;
>       char *name = NULL;
> @@ -3866,15 +3817,9 @@ prlsdkFindNetByPath(virDomainObjPtr dom, const char *path)
>           pret = PrlVmCfg_GetNetAdapter(privdom->sdkdom, i, &net);
>           prlsdkCheckRetGoto(pret, error);
>   
> -        pret = PrlVmDevNet_GetHostInterfaceName(net, NULL, &buflen);
> -        prlsdkCheckRetGoto(pret, error);
> -
> -        if (VIR_ALLOC_N(name, buflen) < 0)
> +        if (!(name = PrlVmDevNet_GetHostInterfaceName_Alloc(net)))
>               goto error;
>   
> -        pret = PrlVmDevNet_GetHostInterfaceName(net, name, &buflen);
> -        prlsdkCheckRetGoto(pret, error);
> -
>           if (STREQ(name, path))
>               break;
>   

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list