Re: [libvirt PATCH 2/2] esx: switch esxUtil_ResolveHostname to return a new string

[Laine Stump <laine@xxxxxxxxxx>]

On 10/5/20 7:41 AM, Pino Toscano wrote:
> Change the interface of esxUtil_ResolveHostname() to return a newly
> allocated string with the result address, instead of forcing the callers
> to provide a buffer and its size. This way we can simply (auto)free the
> string, and make the function stacks smaller.
>
> Signed-off-by: Pino Toscano <ptoscano@xxxxxxxxxx>


Reviewed-by: Laine Stump <laine@xxxxxxxxxx>


> ---
>   src/esx/esx_driver.c | 20 +++++++-------------
>   src/esx/esx_util.c   | 11 +++++++----
>   src/esx/esx_util.h   |  3 +--
>   3 files changed, 15 insertions(+), 19 deletions(-)
>
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index e82e5ed835..a17bf58a51 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -602,7 +602,7 @@ esxConnectToHost(esxPrivate *priv,
>                    char **vCenterIPAddress)
>   {
>       int result = -1;
> -    char ipAddress[NI_MAXHOST] = "";
> +    g_autofree char *ipAddress = NULL;
>       char *username = NULL;
>       char *password = NULL;
>       char *url = NULL;
> @@ -615,7 +615,7 @@ esxConnectToHost(esxPrivate *priv,
>   
>       ESX_VI_CHECK_ARG_LIST(vCenterIPAddress);
>   
> -    if (esxUtil_ResolveHostname(conn->uri->server, ipAddress, NI_MAXHOST) < 0)
> +    if (esxUtil_ResolveHostname(conn->uri->server, &ipAddress) < 0)
>           return -1;
>   
>       if (conn->uri->user) {
> @@ -692,7 +692,7 @@ esxConnectToVCenter(esxPrivate *priv,
>                       const char *hostSystemIPAddress)
>   {
>       int result = -1;
> -    char ipAddress[NI_MAXHOST] = "";
> +    g_autofree char *ipAddress = NULL;
>       char *username = NULL;
>       char *password = NULL;
>       char *url = NULL;
> @@ -704,7 +704,7 @@ esxConnectToVCenter(esxPrivate *priv,
>           return -1;
>       }
>   
> -    if (esxUtil_ResolveHostname(hostname, ipAddress, NI_MAXHOST) < 0)
> +    if (esxUtil_ResolveHostname(hostname, &ipAddress) < 0)
>           return -1;
>   
>       if (conn->uri->user) {
> @@ -813,7 +813,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>       virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
>       esxPrivate *priv = NULL;
>       char *potentialVCenterIPAddress = NULL;
> -    char vCenterIPAddress[NI_MAXHOST] = "";
> +    g_autofree char *vCenterIPAddress = NULL;
>   
>       virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
>   
> @@ -875,16 +875,10 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>                       goto cleanup;
>                   }
>   
> -                if (virStrcpyStatic(vCenterIPAddress,
> -                                    potentialVCenterIPAddress) < 0) {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   _("vCenter IP address %s too big for destination"),
> -                                   potentialVCenterIPAddress);
> -                    goto cleanup;
> -                }
> +                vCenterIPAddress = g_strdup(potentialVCenterIPAddress);
>               } else {
>                   if (esxUtil_ResolveHostname(priv->parsedUri->vCenter,
> -                                            vCenterIPAddress, NI_MAXHOST) < 0) {
> +                                            &vCenterIPAddress) < 0) {
>                       goto cleanup;
>                   }
>   
> diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
> index 555158f953..d9e7641d67 100644
> --- a/src/esx/esx_util.c
> +++ b/src/esx/esx_util.c
> @@ -278,12 +278,12 @@ esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName,
>   
>   
>   int
> -esxUtil_ResolveHostname(const char *hostname,
> -                        char *ipAddress, size_t ipAddress_length)
> +esxUtil_ResolveHostname(const char *hostname, char **ipAddress)
>   {
>       struct addrinfo hints;
>       struct addrinfo *result = NULL;
>       int errcode;
> +    g_autofree char *address = NULL;
>   
>       memset(&hints, 0, sizeof(hints));
>   
> @@ -308,8 +308,9 @@ esxUtil_ResolveHostname(const char *hostname,
>           return -1;
>       }
>   
> -    errcode = getnameinfo(result->ai_addr, result->ai_addrlen, ipAddress,
> -                          ipAddress_length, NULL, 0, NI_NUMERICHOST);
> +    address = g_new0(char, NI_MAXHOST);
> +    errcode = getnameinfo(result->ai_addr, result->ai_addrlen, address,
> +                          NI_MAXHOST, NULL, 0, NI_NUMERICHOST);
>       freeaddrinfo(result);
>   
>       if (errcode != 0) {
> @@ -319,6 +320,8 @@ esxUtil_ResolveHostname(const char *hostname,
>           return -1;
>       }
>   
> +    *ipAddress = g_strdup(address);
> +
>       return 0;
>   }
>   
> diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h
> index 97b6d82a2b..9bfbff1d42 100644
> --- a/src/esx/esx_util.h
> +++ b/src/esx/esx_util.h
> @@ -55,8 +55,7 @@ int esxUtil_ParseVirtualMachineIDString(const char *id_string, int *id);
>   int esxUtil_ParseDatastorePath(const char *datastorePath, char **datastoreName,
>                                  char **directoryName, char **directoryAndFileName);
>   
> -int esxUtil_ResolveHostname(const char *hostname,
> -                            char *ipAddress, size_t ipAddress_length);
> +int esxUtil_ResolveHostname(const char *hostname, char **ipAddress);
>   
>   int esxUtil_ReformatUuid(const char *input, char *output);
>