| Message ID | 20230928132019.2544702-23-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/rdma: Error handling fixes | expand |
Markus Armbruster <armbru@redhat.com> writes: > 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 > merely a latent bug. Drop the flawed code handling null argument. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de>
Markus Armbruster <armbru@redhat.com> 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 > merely a latent bug. Drop the flawed code handling null argument. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> As your discussion with Peter, I think that this is ok. The only other thing we could do is add an assert(), but I am not a big fan either.
On 28/09/2023 21:19, 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 > merely a latent bug. Drop the flawed code handling null argument. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > --- > migration/rdma.c | 29 +++++++++++++---------------- > 1 file changed, 13 insertions(+), 16 deletions(-) > > diff --git a/migration/rdma.c b/migration/rdma.c > index 1a0ad44411..1ae2f87906 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -2747,25 +2747,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; > } >
diff --git a/migration/rdma.c b/migration/rdma.c index 1a0ad44411..1ae2f87906 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -2747,25 +2747,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; }
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 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(-)