Re: [PATCH 20/52] migration/rdma: Drop dead qemu_rdma_data_init() code for !@host_port

[Peter Xu]

On Mon, Sep 18, 2023 at 04:41:34PM +0200, Markus Armbruster wrote:
> qemu_rdma_data_init() neglects to set an Error when it fails because
> @host_port is null.  Fortunately, no caller passes null, so this is

Indeed they all seem to be non-null.

Before this patch, qemu_rdma_data_init() can still tolerant NULL, not
setting errp but still returning NULL showing an error.

After this patch, qemu_rdma_data_init() should crash at inet_parse() if
it's null.

Would it be simpler and clearer if we just set ERROR() for !host_port?

Thanks,

> merely a latent bug.  Drop the flawed code handling null argument.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  migration/rdma.c | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index d3dc162363..cc59155a50 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2716,25 +2716,22 @@ static RDMAContext *qemu_rdma_data_init(const char 
> *host_port, Error **errp)
>      RDMAContext *rdma = NULL;
>      InetSocketAddress *addr;
>  
> -    if (host_port) {
> -        rdma = g_new0(RDMAContext, 1);
> -        rdma->current_index = -1;
> -        rdma->current_chunk = -1;
> +    rdma = g_new0(RDMAContext, 1);
> +    rdma->current_index = -1;
> +    rdma->current_chunk = -1;
>  
> -        addr = g_new(InetSocketAddress, 1);
> -        if (!inet_parse(addr, host_port, NULL)) {
> -            rdma->port = atoi(addr->port);
> -            rdma->host = g_strdup(addr->host);
> -            rdma->host_port = g_strdup(host_port);
> -        } else {
> -            ERROR(errp, "bad RDMA migration address '%s'", host_port);
> -            g_free(rdma);
> -            rdma = NULL;
> -        }
> -
> -        qapi_free_InetSocketAddress(addr);
> +    addr = g_new(InetSocketAddress, 1);
> +    if (!inet_parse(addr, host_port, NULL)) {
> +        rdma->port = atoi(addr->port);
> +        rdma->host = g_strdup(addr->host);
> +        rdma->host_port = g_strdup(host_port);
> +    } else {
> +        ERROR(errp, "bad RDMA migration address '%s'", host_port);
> +        g_free(rdma);
> +        rdma = NULL;
>      }
>  
> +    qapi_free_InetSocketAddress(addr);
>      return rdma;
>  }
>  
> -- 
> 2.41.0
> 

-- 
Peter Xu