Re: [PATCH v3 13/13] migration/rdma: Remove all "ret" variables that are used only once

[Fabiano Rosas]

Juan Quintela <quintela@redhat.com> writes:

> Change code that is:
>
> int ret;
> ...
>
> ret = foo();
> if (ret[ < 0]?) {
>
> to:
>
> if (foo()[ < 0]) {
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/rdma.c | 29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index a43527a83c..c382588b26 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1107,7 +1107,6 @@ err_alloc_pd_cq:
>  static int qemu_rdma_alloc_qp(RDMAContext *rdma)
>  {
>      struct ibv_qp_init_attr attr = { 0 };
> -    int ret;
>  
>      attr.cap.max_send_wr = RDMA_SIGNALED_SEND_MAX;
>      attr.cap.max_recv_wr = 3;
> @@ -1117,8 +1116,7 @@ static int qemu_rdma_alloc_qp(RDMAContext *rdma)
>      attr.recv_cq = rdma->recv_cq;
>      attr.qp_type = IBV_QPT_RC;
>  
> -    ret = rdma_create_qp(rdma->cm_id, rdma->pd, &attr);
> -    if (ret < 0) {
> +    if (rdma_create_qp(rdma->cm_id, rdma->pd, &attr) < 0) {

This particular pattern hurts readability IMO. See how the < 0 got
pushed all the way to the end of the line. The longer the list of
arguments, the larger the chance of missing the < 0 when glancing over
the code.

Anyway:

Reviewed-by: Fabiano Rosas <farosas@suse.de>