On Thu, Oct 12, 2017 at 2:55 PM, Christos Gkekas <chris.gekas@xxxxxxxxx> wrote:
> Remove variables in mds_client that are set but never used.
Hmm, it looks like all the values removed here (except err) *are* used
and this patch would break the world. Specifically...
>
> Signed-off-by: Christos Gkekas <chris.gekas@xxxxxxxxx>
> ---
> fs/ceph/mds_client.c | 11 -----------
> 1 file changed, 11 deletions(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index f23c820..cadf9b6 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3797,11 +3797,8 @@ void ceph_mdsc_handle_fsmap(struct ceph_mds_client *mdsc, struct ceph_msg *msg)
> void *p = msg->front.iov_base;
> void *end = p + msg->front.iov_len;
> u32 epoch;
> - u32 map_len;
> u32 num_fs;
> u32 mount_fscid = (u32)-1;
> - u8 struct_v, struct_cv;
> - int err = -EINVAL;
>
> ceph_decode_need(&p, end, sizeof(u32), bad);
> epoch = ceph_decode_32(&p);
> @@ -3809,10 +3806,6 @@ void ceph_mdsc_handle_fsmap(struct ceph_mds_client *mdsc, struct ceph_msg *msg)
> dout("handle_fsmap epoch %u\n", epoch);
>
> ceph_decode_need(&p, end, 2 + sizeof(u32), bad);
> - struct_v = ceph_decode_8(&p);
> - struct_cv = ceph_decode_8(&p);
> - map_len = ceph_decode_32(&p);
> -
p here is a pointer into a buffer which was sent to us over the
network (msg->front.iov_base, shown at top of patch). ceph_decode is
reading data out of that buffer into these variables and advancing p
for the subsequent readers. If you drop these statements, we start
trying to read values at the wrong offsets, and obviously nothing
makes sense.
> ceph_decode_need(&p, end, sizeof(u32) * 3, bad);
> p += sizeof(u32) * 2; /* skip epoch and legacy_client_fscid */
>
> @@ -3820,12 +3813,9 @@ void ceph_mdsc_handle_fsmap(struct ceph_mds_client *mdsc, struct ceph_msg *msg)
> while (num_fs-- > 0) {
> void *info_p, *info_end;
> u32 info_len;
> - u8 info_v, info_cv;
> u32 fscid, namelen;
>
> ceph_decode_need(&p, end, 2 + sizeof(u32), bad);
> - info_v = ceph_decode_8(&p);
> - info_cv = ceph_decode_8(&p);
> info_len = ceph_decode_32(&p);
> ceph_decode_need(&p, end, info_len, bad);
> info_p = p;
> @@ -3852,7 +3842,6 @@ void ceph_mdsc_handle_fsmap(struct ceph_mds_client *mdsc, struct ceph_msg *msg)
> 0, true);
> ceph_monc_renew_subs(&fsc->client->monc);
> } else {
> - err = -ENOENT;
> goto err_out;
Amusingly, this bit does seem to be correct, as the err_out block just
unconditionally sets mdsc->mdsmap_err to -ENOENT (and is only
reachable from this goto).
> }
> return;
I assume this patch was generated by some code cleanliness tool, so it
appears to need some work... :)
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
