On 18/09/2023 22:41, Markus Armbruster wrote:
> rdma_getaddrinfo() returns 0 on success. On error, it returns one of
> the EAI_ error codes like getaddrinfo() does,
Good catch, It used to be -1 on error, rdma_getaddrinfo(3) updated it 2021.
or -1 with errno set.
> This is broken by design: POSIX implicitly specifies the EAI_ error
> codes to be non-zero, no more. They could clash with -1. Nothing we
> can do about this design flaw.
>
> Both callers of rdma_getaddrinfo() only recognize negative values as
> error. Works only because systems elect to make the EAI_ error codes
> negative.
>
> Best not to rely on that: change the callers to treat any non-zero
> value as failure. Also change them to return -1 instead of the value
> received from getaddrinfo() on failure, to avoid positive error
> values.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
Reviewed-by: Li Zhijian <lizhij...@fujitsu.com>
> ---
> migration/rdma.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 46b5859268..3421ae0796 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -935,14 +935,14 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma,
> Error **errp)
>
> if (rdma->host == NULL || !strcmp(rdma->host, "")) {
> ERROR(errp, "RDMA hostname has not been set");
> - return -EINVAL;
> + return -1;
> }
>
> /* create CM channel */
> rdma->channel = rdma_create_event_channel();
> if (!rdma->channel) {
> ERROR(errp, "could not create CM channel");
> - return -EINVAL;
> + return -1;
> }
>
> /* create CM id */
> @@ -956,7 +956,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma,
> Error **errp)
> port_str[15] = '\0';
>
> ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> - if (ret < 0) {
> + if (ret) {
> ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> goto err_resolve_get_addr;
> }
> @@ -998,7 +998,6 @@ route:
> rdma_event_str(cm_event->event));
> error_report("rdma_resolve_addr");
> rdma_ack_cm_event(cm_event);
> - ret = -EINVAL;
> goto err_resolve_get_addr;
> }
> rdma_ack_cm_event(cm_event);
> @@ -1019,7 +1018,6 @@ route:
> ERROR(errp, "result not equal to event_route_resolved: %s",
> rdma_event_str(cm_event->event));
> rdma_ack_cm_event(cm_event);
> - ret = -EINVAL;
> goto err_resolve_get_addr;
> }
> rdma_ack_cm_event(cm_event);
> @@ -1034,7 +1032,7 @@ err_resolve_get_addr:
> err_resolve_create_id:
> rdma_destroy_event_channel(rdma->channel);
> rdma->channel = NULL;
> - return ret;
> + return -1;
> }
>
> /*
> @@ -2644,7 +2642,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error
> **errp)
> port_str[15] = '\0';
>
> ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> - if (ret < 0) {
> + if (ret) {
> ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> goto err_dest_init_bind_addr;
> }
> @@ -2688,7 +2686,7 @@ err_dest_init_create_listen_id:
> rdma_destroy_event_channel(rdma->channel);
> rdma->channel = NULL;
> rdma->error_state = ret;
> - return ret;
> + return -1;
>
> }
>
