On Wed, Jul 12, 2017 at 03:17:38PM -0600, Jason Gunthorpe wrote:
> Sparse says:
> warning: dubious: !x & y
>
> Unclear why sparse thinks this is bad, but casting to C99 bool is the
> same as !! and much easier to read.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@xxxxxxxxxxxxxxxxxxxx>
> ---
> providers/mlx5/cq.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/providers/mlx5/cq.c b/providers/mlx5/cq.c
> index 500116133c1f9e..9a8d958a9ced68 100644
> --- a/providers/mlx5/cq.c
> +++ b/providers/mlx5/cq.c
> @@ -206,11 +206,12 @@ static inline int handle_responder(struct ibv_wc *wc, struct mlx5_cqe64 *cqe,
> if (likely(cur_rsc->type == MLX5_RSC_TYPE_QP)) {
> wq = &qp->rq;
> if (qp->qp_cap_cache & MLX5_RX_CSUM_VALID)
> - wc->wc_flags |= (!!(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> - !!(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> - (get_cqe_l3_hdr_type(cqe) ==
> - MLX5_CQE_L3_HDR_TYPE_IPV4)) <<
> - IBV_WC_IP_CSUM_OK_SHIFT;
> + wc->wc_flags |=
> + ((bool)(cqe->hds_ip_ext & MLX5_CQE_L4_OK) &
> + (bool)(cqe->hds_ip_ext & MLX5_CQE_L3_OK) &
> + (get_cqe_l3_hdr_type(cqe) ==
> + MLX5_CQE_L3_HDR_TYPE_IPV4))
> + << IBV_WC_IP_CSUM_OK_SHIFT;
Meh. This code is complete crap. Please factor it out into a little
helper that mere humans can read first. And then replace the odd ^ used
as && with proper if constructs and all should make much more sense.
Something like:
static bool mlx4_ipv4_csum_ok(sruct mlx5_cqe64 *cqe)
{
if (get_cqe_l3_hdr_type(cqe) != MLX5_CQE_L3_HDR_TYPE_IPV4)
return false;
if (!(cqe->hds_ip_ext & MLX5_CQE_L4_OK))
return false;
if (!(cqe->hds_ip_ext & MLX5_CQE_L3_OK))
return false;
return true;
}
...
if (mlx4_ipv4_csum_ok(cqe))
wc->wc_flags |= 1 << IBV_WC_IP_CSUM_OK_SHIFT;
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
