Hi Dan-
> On Jul 14, 2017, at 3:33 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> Hello Chuck Lever,
>
> The patch 026d958b38c6: "svcrdma: Add recvfrom helpers to
> svc_rdma_rw.c" from Jun 23, 2017, leads to the following static
> checker warning:
>
> net/sunrpc/xprtrdma/svc_rdma_rw.c:689 svc_rdma_build_read_chunk()
> error: uninitialized symbol 'ret'.
Interesting. I often build individual files with "make W=1"
as part of normal development. I have never seen this warning
in this function.
> net/sunrpc/xprtrdma/svc_rdma_rw.c
> 663 static int svc_rdma_build_read_chunk(struct svc_rqst *rqstp,
> 664 struct svc_rdma_read_info *info,
> 665 __be32 *p)
> 666 {
> 667 int ret;
> 668
> 669 info->ri_chunklen = 0;
> 670 while (*p++ != xdr_zero) {
> ^^^^^^^^^^^^^^^^
> Is this necessarily true for the first iteration through the loop?
Yes. This function is called only when there is a Read chunk
to process starting at @p. There will always be at least one
segment.
Constructing the loop this way eliminates some code duplication,
at the cost of confusing human readers and static checkers.
> 671 u32 rs_handle, rs_length;
> 672 u64 rs_offset;
> 673
> 674 if (be32_to_cpup(p++) != info->ri_position)
> 675 break;
> ^^^^^
> It feels like this should be -EINVAL.
No. The loop terminates normally here if the Position value
changes. That signals the start of the next Read chunk. This
is not an error, but meeting the condition will be rather
rare.
In fact, xdr_check_read_list prevents there from ever being
a subsequent Read chunk, since this server can't handle more
than one, currently. I could safely remove this check for now.
But during the first loop iteration, this condition is never
met. The loop body will already have been executed at least
once, so "ret" is already set by svc_rdma_build_read_segment.
> 676 rs_handle = be32_to_cpup(p++);
> 677 rs_length = be32_to_cpup(p++);
> 678 p = xdr_decode_hyper(p, &rs_offset);
> 679
> 680 ret = svc_rdma_build_read_segment(info, rqstp,
> 681 rs_handle, rs_length,
> 682 rs_offset);
> 683 if (ret < 0)
> 684 break;
> 685
> 686 info->ri_chunklen += rs_length;
> 687 }
> 688
> 689 return ret;
> 690 }
>
> 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
