Re: [PATCH 1/1 net-next] libceph: remove unused variable in handle_reply()

On 23 October 2014 at 18:25 Ilya Dryomov [off-list ref] wrote:


On Thu, Oct 23, 2014 at 8:15 PM, Fabian Frederick [off-list ref] wrote:

quoted

osdmap_epoch is redundant with reassert_epoch and unused.

Signed-off-by: Fabian Frederick <redacted>
---
 net/ceph/osd_client.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index f3fc54e..432bd75 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c

@@ -1700,7 +1700,6 @@ static void handle_reply(struct ceph_osd_client *osdc,

struct ceph_msg *msg,
        int err;
        u32 reassert_epoch;
        u64 reassert_version;
-       u32 osdmap_epoch;
        int already_completed;
        u32 bytes;
        unsigned int i;

@@ -1725,7 +1724,6 @@ static void handle_reply(struct ceph_osd_client *osdc,

struct ceph_msg *msg,
        result = ceph_decode_32(&p);
        reassert_epoch = ceph_decode_32(&p);
        reassert_version = ceph_decode_64(&p);
-       osdmap_epoch = ceph_decode_32(&p);

        /* lookup */
        down_read(&osdc->map_sem);

Hi Fabian,

osdmap_epoch is useful for debugging, but this is wrong anyway -
ceph_decode_32() has side effects.  Removing it and not adjusting *p
would make the whole thing blow up pretty fast..

Thanks,

                Ilya

Hi Ilya,

        osdmap_epoch generates a warning with -Wunused-but-set-variable.
Maybe we could just do ceph_decode_32(&p) and remove it
(it doesn't seem to add a warning) and/or add some comment ?

Regards,
Fabian