This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Storable.xs: introduce SEEN*_NN
authorDavid Mitchell <davem@iabyn.com>
Tue, 16 Dec 2014 16:57:25 +0000 (16:57 +0000)
committerDavid Mitchell <davem@iabyn.com>
Tue, 16 Dec 2014 16:57:25 +0000 (16:57 +0000)
Introduce _NN versions of the SEEN() and SEEN0() macros, that
don't bother to check whether their first arg is null.

The initial motivation for this was to to silence a compiler warning
about a conditional always being true in SEEN(&PL_sv_undef,...), but
I've also applied it to all places in the code where it's clear that the
arg can't be null, e.g.

    sv = NEWSV();
    SEEN(sv,...);

At it happens, "places where" currently equates to every usage of
SEEN/SEEN0 in the source code.

dist/Storable/Storable.xs

index b59c580..7d27275 100644 (file)
@@ -1038,24 +1038,38 @@ static const char byteorderstr_56[] = {BYTEORDER_BYTES_56, 0};
  * i should be true iff sv is immortal (ie PL_sv_yes, PL_sv_no or PL_sv_undef)
  *
  * SEEN0() is a short-cut where stash is always NULL.
+ *
+ * The _NN variants dont check for y being null
  */
-#define SEEN0(y,i)                                                     \
+#define SEEN0_NN(y,i)                                                  \
     STMT_START {                                                       \
-       if (!y)                                                         \
-               return (SV *) 0;                                        \
        if (av_store(cxt->aseen, cxt->tagnum++, i ? (SV*)(y) : SvREFCNT_inc(y)) == 0) \
                return (SV *) 0;                                        \
        TRACEME(("aseen(#%d) = 0x%"UVxf" (refcnt=%d)", cxt->tagnum-1,   \
                 PTR2UV(y), SvREFCNT(y)-1));                            \
     } STMT_END
 
-#define SEEN(y,stash,i)                                                        \
+#define SEEN0(y,i)                                                     \
+    STMT_START {                                                       \
+       if (!y)                                                         \
+               return (SV *) 0;                                        \
+        SEEN0_NN(y,i)                                                  \
+    } STMT_END
+
+#define SEEN_NN(y,stash,i)                                             \
     STMT_START {                                                       \
-        SEEN0(y,i);                                                    \
+        SEEN0_NN(y,i);                                                 \
        if (stash)                                                      \
                BLESS((SV *) (y), (HV *)(stash));                       \
     } STMT_END
 
+#define SEEN(y,stash,i)                                                        \
+    STMT_START {                                                       \
+       if (!y)                                                         \
+           return (SV *) 0;                                            \
+        SEEN_NN(y,stash, i);                                           \
+    } STMT_END
+
 /*
  * Bless 's' in 'p', via a temporary reference, required by sv_bless().
  * "A" magic is added before the sv_bless for overloaded classes, this avoids
@@ -4159,7 +4173,7 @@ static SV *retrieve_hook(pTHX_ stcxt_t *cxt, const char *cname)
        default:
                return retrieve_other(aTHX_ cxt, 0);            /* Let it croak */
        }
-       SEEN0(sv, 0);                                                   /* Don't bless yet */
+       SEEN0_NN(sv, 0);                                                        /* Don't bless yet */
 
        /*
         * Whilst flags tell us to recurse, do so.
@@ -4356,7 +4370,7 @@ static SV *retrieve_hook(pTHX_ stcxt_t *cxt, const char *cname)
                SvREFCNT_dec(sv);
                /* we need to free RV but preserve value that RV point to */
                sv = SvRV(attached);
-               SEEN0(sv, 0);
+               SEEN0_NN(sv, 0);
                SvRV_set(attached, NULL);
                SvREFCNT_dec(attached);
                if (!(flags & SHF_IDX_CLASSNAME) && classname != buf)
@@ -4531,7 +4545,7 @@ static SV *retrieve_ref(pTHX_ stcxt_t *cxt, const char *cname)
                stash = gv_stashpv(cname, GV_ADD);
        else
                stash = 0;
-       SEEN(rv, stash, 0);                             /* Will return if rv is null */
+       SEEN_NN(rv, stash, 0);                          /* Will return if rv is null */
        sv = retrieve(aTHX_ cxt, 0);    /* Retrieve <object> */
        if (!sv)
                return (SV *) 0;        /* Failed */
@@ -4611,7 +4625,7 @@ static SV *retrieve_overloaded(pTHX_ stcxt_t *cxt, const char *cname)
 
        rv = NEWSV(10002, 0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(rv, stash, 0);             /* Will return if rv is null */
+       SEEN_NN(rv, stash, 0);          /* Will return if rv is null */
        cxt->in_retrieve_overloaded = 1; /* so sv_bless doesn't call S_reset_amagic */
        sv = retrieve(aTHX_ cxt, 0);    /* Retrieve <object> */
        cxt->in_retrieve_overloaded = 0;
@@ -4697,7 +4711,7 @@ static SV *retrieve_tied_array(pTHX_ stcxt_t *cxt, const char *cname)
 
        tv = NEWSV(10002, 0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(tv, stash, 0);                     /* Will return if tv is null */
+       SEEN_NN(tv, stash, 0);                  /* Will return if tv is null */
        sv = retrieve(aTHX_ cxt, 0);            /* Retrieve <object> */
        if (!sv)
                return (SV *) 0;                /* Failed */
@@ -4728,7 +4742,7 @@ static SV *retrieve_tied_hash(pTHX_ stcxt_t *cxt, const char *cname)
 
        tv = NEWSV(10002, 0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(tv, stash, 0);                     /* Will return if tv is null */
+       SEEN_NN(tv, stash, 0);                  /* Will return if tv is null */
        sv = retrieve(aTHX_ cxt, 0);            /* Retrieve <object> */
        if (!sv)
                return (SV *) 0;                /* Failed */
@@ -4758,7 +4772,7 @@ static SV *retrieve_tied_scalar(pTHX_ stcxt_t *cxt, const char *cname)
 
        tv = NEWSV(10002, 0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(tv, stash, 0);                     /* Will return if rv is null */
+       SEEN_NN(tv, stash, 0);                  /* Will return if rv is null */
        sv = retrieve(aTHX_ cxt, 0);            /* Retrieve <object> */
        if (!sv) {
                return (SV *) 0;                /* Failed */
@@ -4797,7 +4811,7 @@ static SV *retrieve_tied_key(pTHX_ stcxt_t *cxt, const char *cname)
 
        tv = NEWSV(10002, 0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(tv, stash, 0);                     /* Will return if tv is null */
+       SEEN_NN(tv, stash, 0);                  /* Will return if tv is null */
        sv = retrieve(aTHX_ cxt, 0);            /* Retrieve <object> */
        if (!sv)
                return (SV *) 0;                /* Failed */
@@ -4831,7 +4845,7 @@ static SV *retrieve_tied_idx(pTHX_ stcxt_t *cxt, const char *cname)
 
        tv = NEWSV(10002, 0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(tv, stash, 0);                     /* Will return if tv is null */
+       SEEN_NN(tv, stash, 0);                  /* Will return if tv is null */
        sv = retrieve(aTHX_ cxt, 0);            /* Retrieve <object> */
        if (!sv)
                return (SV *) 0;                /* Failed */
@@ -4870,7 +4884,7 @@ static SV *retrieve_lscalar(pTHX_ stcxt_t *cxt, const char *cname)
 
        sv = NEWSV(10002, len);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);     /* Associate this new scalar with tag "tagnum" */
+       SEEN_NN(sv, stash, 0);  /* Associate this new scalar with tag "tagnum" */
 
        if (len ==  0) {
            sv_setpvn(sv, "", 0);
@@ -4923,7 +4937,7 @@ static SV *retrieve_scalar(pTHX_ stcxt_t *cxt, const char *cname)
 
        sv = NEWSV(10002, len);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);     /* Associate this new scalar with tag "tagnum" */
+       SEEN_NN(sv, stash, 0);  /* Associate this new scalar with tag "tagnum" */
 
        /*
         * WARNING: duplicates parts of sv_setpv and breaks SV data encapsulation.
@@ -5108,7 +5122,7 @@ static SV *retrieve_integer(pTHX_ stcxt_t *cxt, const char *cname)
        READ(&iv, sizeof(iv));
        sv = newSViv(iv);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);     /* Associate this new scalar with tag "tagnum" */
+       SEEN_NN(sv, stash, 0);  /* Associate this new scalar with tag "tagnum" */
 
        TRACEME(("integer %"IVdf, iv));
        TRACEME(("ok (retrieve_integer at 0x%"UVxf")", PTR2UV(sv)));
@@ -5139,7 +5153,7 @@ static SV *retrieve_netint(pTHX_ stcxt_t *cxt, const char *cname)
        TRACEME(("network integer (as-is) %d", iv));
 #endif
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);     /* Associate this new scalar with tag "tagnum" */
+       SEEN_NN(sv, stash, 0);  /* Associate this new scalar with tag "tagnum" */
 
        TRACEME(("ok (retrieve_netint at 0x%"UVxf")", PTR2UV(sv)));
 
@@ -5163,7 +5177,7 @@ static SV *retrieve_double(pTHX_ stcxt_t *cxt, const char *cname)
        READ(&nv, sizeof(nv));
        sv = newSVnv(nv);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);     /* Associate this new scalar with tag "tagnum" */
+       SEEN_NN(sv, stash, 0);  /* Associate this new scalar with tag "tagnum" */
 
        TRACEME(("double %"NVff, nv));
        TRACEME(("ok (retrieve_double at 0x%"UVxf")", PTR2UV(sv)));
@@ -5191,7 +5205,7 @@ static SV *retrieve_byte(pTHX_ stcxt_t *cxt, const char *cname)
        tmp = (unsigned char) siv - 128;
        sv = newSViv(tmp);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);     /* Associate this new scalar with tag "tagnum" */
+       SEEN_NN(sv, stash, 0);  /* Associate this new scalar with tag "tagnum" */
 
        TRACEME(("byte %d", tmp));
        TRACEME(("ok (retrieve_byte at 0x%"UVxf")", PTR2UV(sv)));
@@ -5213,7 +5227,7 @@ static SV *retrieve_undef(pTHX_ stcxt_t *cxt, const char *cname)
 
        sv = newSV(0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);
+       SEEN_NN(sv, stash, 0);
 
        return sv;
 }
@@ -5237,7 +5251,7 @@ static SV *retrieve_sv_undef(pTHX_ stcxt_t *cxt, const char *cname)
                cxt->where_is_undef = cxt->tagnum;
        }
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 1);
+       SEEN_NN(sv, stash, 1);
        return sv;
 }
 
@@ -5254,7 +5268,7 @@ static SV *retrieve_sv_yes(pTHX_ stcxt_t *cxt, const char *cname)
        TRACEME(("retrieve_sv_yes"));
 
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 1);
+       SEEN_NN(sv, stash, 1);
        return sv;
 }
 
@@ -5271,7 +5285,7 @@ static SV *retrieve_sv_no(pTHX_ stcxt_t *cxt, const char *cname)
        TRACEME(("retrieve_sv_no"));
 
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 1);
+       SEEN_NN(sv, stash, 1);
        return sv;
 }
 
@@ -5288,7 +5302,7 @@ static SV *retrieve_svundef_elem(pTHX_ stcxt_t *cxt, const char *cname)
 
        /* SEEN reads the contents of its SV argument, which we are not
           supposed to do with &PL_sv_placeholder. */
-       SEEN(&PL_sv_undef, cname, 1);
+       SEEN_NN(&PL_sv_undef, cname, 1);
 
        return &PL_sv_placeholder;
 }
@@ -5321,7 +5335,7 @@ static SV *retrieve_array(pTHX_ stcxt_t *cxt, const char *cname)
        TRACEME(("size = %d", len));
        av = newAV();
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(av, stash, 0);                     /* Will return if array not allocated nicely */
+       SEEN_NN(av, stash, 0);                  /* Will return if array not allocated nicely */
        if (len)
                av_extend(av, len);
        else
@@ -5382,7 +5396,7 @@ static SV *retrieve_hash(pTHX_ stcxt_t *cxt, const char *cname)
        TRACEME(("size = %d", len));
        hv = newHV();
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(hv, stash, 0);             /* Will return if table not allocated properly */
+       SEEN_NN(hv, stash, 0);          /* Will return if table not allocated properly */
        if (len == 0)
                return (SV *) hv;       /* No data follow if table empty */
        hv_ksplit(hv, len + 1);         /* pre-extend hash to save multiple splits */
@@ -5471,7 +5485,7 @@ static SV *retrieve_flag_hash(pTHX_ stcxt_t *cxt, const char *cname)
     TRACEME(("size = %d, flags = %d", len, hash_flags));
     hv = newHV();
     stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-    SEEN(hv, stash, 0);                /* Will return if table not allocated properly */
+    SEEN_NN(hv, stash, 0);             /* Will return if table not allocated properly */
     if (len == 0)
         return (SV *) hv;      /* No data follow if table empty */
     hv_ksplit(hv, len + 1);            /* pre-extend hash to save multiple splits */
@@ -5601,7 +5615,7 @@ static SV *retrieve_code(pTHX_ stcxt_t *cxt, const char *cname)
        tagnum = cxt->tagnum;
        sv = newSViv(0);
        stash = cname ? gv_stashpv(cname, GV_ADD) : 0;
-       SEEN(sv, stash, 0);
+       SEEN_NN(sv, stash, 0);
 
        /*
         * Retrieve the source of the code reference
@@ -5729,7 +5743,7 @@ static SV *old_retrieve_array(pTHX_ stcxt_t *cxt, const char *cname)
        RLEN(len);
        TRACEME(("size = %d", len));
        av = newAV();
-       SEEN0(av, 0);                           /* Will return if array not allocated nicely */
+       SEEN0_NN(av, 0);                        /* Will return if array not allocated nicely */
        if (len)
                av_extend(av, len);
        else
@@ -5792,7 +5806,7 @@ static SV *old_retrieve_hash(pTHX_ stcxt_t *cxt, const char *cname)
        RLEN(len);
        TRACEME(("size = %d", len));
        hv = newHV();
-       SEEN0(hv, 0);                   /* Will return if table not allocated properly */
+       SEEN0_NN(hv, 0);                /* Will return if table not allocated properly */
        if (len == 0)
                return (SV *) hv;       /* No data follow if table empty */
        hv_ksplit(hv, len + 1);         /* pre-extend hash to save multiple splits */