libceph: fixup error handling in osdmap_decode()
authorIlya Dryomov <ilya.dryomov@inktank.com>
Thu, 13 Mar 2014 14:36:14 +0000 (16:36 +0200)
committerSage Weil <sage@inktank.com>
Sat, 5 Apr 2014 04:07:38 +0000 (21:07 -0700)
The existing error handling scheme requires resetting err to -EINVAL
prior to calling any ceph_decode_* macro.  This is ugly and fragile,
and there already are a few places where we would return 0 on error,
due to a missing reset.  Fix this by adding a special e_inval label to
be used by all ceph_decode_* macros.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
Reviewed-by: Alex Elder <elder@linaro.org>
net/ceph/osdmap.c

index a82df6ea0749893db37776ddf2096610ea3dc266..298d076eee89ceee44cda17e9f5989134aecbf7e 100644 (file)
@@ -688,36 +688,37 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
 {
        u16 version;
        u32 len, max, i;
-       int err = -EINVAL;
        u32 epoch = 0;
        void *start = *p;
+       int err;
        struct ceph_pg_pool_info *pi;
 
        dout("%s %p to %p len %d\n", __func__, *p, end, (int)(end - *p));
 
-       ceph_decode_16_safe(p, end, version, bad);
+       ceph_decode_16_safe(p, end, version, e_inval);
        if (version > 6) {
                pr_warning("got unknown v %d > 6 of osdmap\n", version);
-               goto bad;
+               goto e_inval;
        }
        if (version < 6) {
                pr_warning("got old v %d < 6 of osdmap\n", version);
-               goto bad;
+               goto e_inval;
        }
 
-       ceph_decode_need(p, end, 2*sizeof(u64)+6*sizeof(u32), bad);
+       ceph_decode_need(p, end, 2*sizeof(u64)+6*sizeof(u32), e_inval);
        ceph_decode_copy(p, &map->fsid, sizeof(map->fsid));
        epoch = map->epoch = ceph_decode_32(p);
        ceph_decode_copy(p, &map->created, sizeof(map->created));
        ceph_decode_copy(p, &map->modified, sizeof(map->modified));
 
-       ceph_decode_32_safe(p, end, max, bad);
+       ceph_decode_32_safe(p, end, max, e_inval);
        while (max--) {
-               ceph_decode_need(p, end, 8 + 2, bad);
-               err = -ENOMEM;
+               ceph_decode_need(p, end, 8 + 2, e_inval);
                pi = kzalloc(sizeof(*pi), GFP_NOFS);
-               if (!pi)
+               if (!pi) {
+                       err = -ENOMEM;
                        goto bad;
+               }
                pi->id = ceph_decode_64(p);
                err = __decode_pool(p, end, pi);
                if (err < 0) {
@@ -728,27 +729,25 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
        }
 
        err = __decode_pool_names(p, end, map);
-       if (err < 0) {
-               dout("fail to decode pool names");
+       if (err)
                goto bad;
-       }
 
-       ceph_decode_32_safe(p, end, map->pool_max, bad);
+       ceph_decode_32_safe(p, end, map->pool_max, e_inval);
 
-       ceph_decode_32_safe(p, end, map->flags, bad);
+       ceph_decode_32_safe(p, end, map->flags, e_inval);
 
        max = ceph_decode_32(p);
 
        /* (re)alloc osd arrays */
        err = osdmap_set_max_osd(map, max);
-       if (err < 0)
+       if (err)
                goto bad;
 
        /* osds */
-       err = -EINVAL;
        ceph_decode_need(p, end, 3*sizeof(u32) +
                         map->max_osd*(1 + sizeof(*map->osd_weight) +
-                                      sizeof(*map->osd_addr)), bad);
+                                      sizeof(*map->osd_addr)), e_inval);
+
        *p += 4; /* skip length field (should match max) */
        ceph_decode_copy(p, map->osd_state, map->max_osd);
 
@@ -762,7 +761,7 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
                ceph_decode_addr(&map->osd_addr[i]);
 
        /* pg_temp */
-       ceph_decode_32_safe(p, end, len, bad);
+       ceph_decode_32_safe(p, end, len, e_inval);
        for (i = 0; i < len; i++) {
                int n, j;
                struct ceph_pg pgid;
@@ -771,16 +770,16 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
                err = ceph_decode_pgid(p, end, &pgid);
                if (err)
                        goto bad;
-               ceph_decode_need(p, end, sizeof(u32), bad);
+               ceph_decode_need(p, end, sizeof(u32), e_inval);
                n = ceph_decode_32(p);
-               err = -EINVAL;
                if (n > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
-                       goto bad;
-               ceph_decode_need(p, end, n * sizeof(u32), bad);
-               err = -ENOMEM;
+                       goto e_inval;
+               ceph_decode_need(p, end, n * sizeof(u32), e_inval);
                pg = kmalloc(sizeof(*pg) + n*sizeof(u32), GFP_NOFS);
-               if (!pg)
+               if (!pg) {
+                       err = -ENOMEM;
                        goto bad;
+               }
                pg->pgid = pgid;
                pg->len = n;
                for (j = 0; j < n; j++)
@@ -794,10 +793,10 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
        }
 
        /* crush */
-       ceph_decode_32_safe(p, end, len, bad);
+       ceph_decode_32_safe(p, end, len, e_inval);
        dout("osdmap_decode crush len %d from off 0x%x\n", len,
             (int)(*p - start));
-       ceph_decode_need(p, end, len, bad);
+       ceph_decode_need(p, end, len, e_inval);
        map->crush = crush_decode(*p, end);
        *p += len;
        if (IS_ERR(map->crush)) {
@@ -812,6 +811,8 @@ static int osdmap_decode(void **p, void *end, struct ceph_osdmap *map)
        dout("full osdmap epoch %d max_osd %d\n", map->epoch, map->max_osd);
        return 0;
 
+e_inval:
+       err = -EINVAL;
 bad:
        pr_err("corrupt full osdmap (%d) epoch %d off %d (%p of %p-%p)\n",
               err, epoch, (int)(*p - start), *p, start, end);