> On Jul 13, 2017, at 10:05 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Chuck Lever,
>
> The patch 107c1d0a991a: "svcrdma: Avoid Send Queue overflow" from Jun
> 23, 2017, leads to the following static checker warning:
>
> net/sunrpc/xprtrdma/svc_rdma_sendto.c:687 svc_rdma_sendto()
> warn: was && intended here instead of ||?
>
> net/sunrpc/xprtrdma/svc_rdma_sendto.c
> 682 if (ret < 0)
> 683 goto err0;
> 684 return 0;
> 685
> 686 err2:
> 687 if (ret != -E2BIG || ret != -EINVAL)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Yeah... This should be &&.
OK.
> But honestly, I'd be so nervous about
> assigning such a specific meaning to -EINVAL. I saw that it's
> documented but I feel like people are always randomly adding -EINVAL
> returns without reading the documentation. I could see myself making
> that mistake.
EINVAL means "the caller provided bad arguments". The documenting
comment you refer to describes a specific case where the function
is catching a "bad argument value". I believe EINVAL is being used
correctly, not narrowly, here.
If you think of a better errno code to use, I'm open to changing
it.
> 688 goto err1;
> 689
> 690 ret = svc_rdma_post_recv(rdma, GFP_KERNEL);
> 691 if (ret)
> 692 goto err1;
> 693 ret = svc_rdma_send_error_msg(rdma, rdma_resp, rqstp);
> 694 if (ret < 0)
> 695 goto err0;
> 696 return 0;
> 697
> 698 err1:
> 699 put_page(res_page);
> 700 err0:
> 701 pr_err("svcrdma: Could not send reply, err=%d. Closing transport.\n",
> 702 ret);
> 703 set_bit(XPT_CLOSE, &xprt->xpt_flags);
> 704 return -ENOTCONN;
> 705 }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
