This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
remove almost unreachable NULL sv arg code from sv_2*n_flags
authorDaniel Dragan <bulk88@hotmail.com>
Fri, 15 Nov 2013 06:52:44 +0000 (01:52 -0500)
committerTony Cook <tony@develop-help.com>
Thu, 28 Nov 2013 04:05:01 +0000 (15:05 +1100)
The NULL sv code being removed dates to commit e334a159a5 Perl 1.0 as
the pre-SV str_2ptr and str_2num calls. When SVs were intoduced in
commit 79072805bf Perl 5.0 alpha 2, the NULL sv code was copied to the new
SV functions. The functions were bulk marked non-NULL in commit f54cb97a39
during 5.9.3 development. The docs were corrected to say NULLOK support
in commit 53e8571218 during 5.11.0.

See the perldelta part of this patch for the rest of commit body.

embed.fnc
mathoms.c
pod/perldelta.pod
proto.h
sv.c

index e98e95b..825ebc9 100644 (file)
--- a/embed.fnc
+++ b/embed.fnc
@@ -1287,19 +1287,19 @@ Apd     |IO*    |sv_2io         |NN SV *const sv
 #if defined(PERL_IN_SV_C)
 s      |bool   |glob_2number   |NN GV* const gv
 #endif
-Amb    |IV     |sv_2iv         |NULLOK SV *sv
-Apd    |IV     |sv_2iv_flags   |NULLOK SV *const sv|const I32 flags
+Amb    |IV     |sv_2iv         |NN SV *sv
+Apd    |IV     |sv_2iv_flags   |NN SV *const sv|const I32 flags
 Apd    |SV*    |sv_2mortal     |NULLOK SV *const sv
-Apd    |NV     |sv_2nv_flags   |NULLOK SV *const sv|const I32 flags
+Apd    |NV     |sv_2nv_flags   |NN SV *const sv|const I32 flags
 : Used in pp.c, pp_hot.c, sv.c
 pMd    |SV*    |sv_2num        |NN SV *const sv
-Amb    |char*  |sv_2pv         |NULLOK SV *sv|NULLOK STRLEN *lp
-Apd    |char*  |sv_2pv_flags   |NULLOK SV *const sv|NULLOK STRLEN *const lp|const I32 flags
+Amb    |char*  |sv_2pv         |NN SV *sv|NULLOK STRLEN *lp
+Apd    |char*  |sv_2pv_flags   |NN SV *const sv|NULLOK STRLEN *const lp|const I32 flags
 Apd    |char*  |sv_2pvutf8     |NN SV *sv|NULLOK STRLEN *const lp
 Apd    |char*  |sv_2pvbyte     |NN SV *sv|NULLOK STRLEN *const lp
 Ap     |char*  |sv_pvn_nomg    |NN SV* sv|NULLOK STRLEN* lp
-Amb    |UV     |sv_2uv         |NULLOK SV *sv
-Apd    |UV     |sv_2uv_flags   |NULLOK SV *const sv|const I32 flags
+Amb    |UV     |sv_2uv         |NN SV *sv
+Apd    |UV     |sv_2uv_flags   |NN SV *const sv|const I32 flags
 Apd    |IV     |sv_iv          |NN SV* sv
 Apd    |UV     |sv_uv          |NN SV* sv
 Apd    |NV     |sv_nv          |NN SV* sv
index b5ae519..0543e88 100644 (file)
--- a/mathoms.c
+++ b/mathoms.c
@@ -148,6 +148,8 @@ Perl_sv_taint(pTHX_ SV *sv)
 IV
 Perl_sv_2iv(pTHX_ SV *sv)
 {
+    PERL_ARGS_ASSERT_SV_2IV;
+
     return sv_2iv_flags(sv, SV_GMAGIC);
 }
 
@@ -158,6 +160,8 @@ Perl_sv_2iv(pTHX_ SV *sv)
 UV
 Perl_sv_2uv(pTHX_ SV *sv)
 {
+    PERL_ARGS_ASSERT_SV_2UV;
+
     return sv_2uv_flags(sv, SV_GMAGIC);
 }
 
@@ -179,6 +183,8 @@ Perl_sv_2nv(pTHX_ SV *sv)
 char *
 Perl_sv_2pv(pTHX_ SV *sv, STRLEN *lp)
 {
+    PERL_ARGS_ASSERT_SV_2PV;
+
     return sv_2pv_flags(sv, lp, SV_GMAGIC);
 }
 
index 6f01e73..e94e6fa 100644 (file)
@@ -334,7 +334,28 @@ well.
 
 =item *
 
-XXX
+The C<sv> argument in L<perlapi/sv_2pv_flags>, L<perlapi/sv_2iv_flags>,
+L<perlapi/sv_2uv_flags>, and L<perlapi/sv_2nv_flags> and their older wrappers
+sv_2pv, sv_2iv, sv_2uv, sv_2nv, is now non-NULL. Passing NULL now will crash.
+When the non-NULL marker was introduced en masse in 5.9.3 the functions
+were marked non-NULL, but since the creation of the SV API in 5.0 alpha 2, if
+NULL was passed, the functions returned 0 or false-type values. The code that
+supports C<sv> argument being non-NULL dates to 5.0 alpha 2 directly, and
+indirectly to Perl 1.0 (pre 5.0 api). The lack of documentation that the
+functions accepted a NULL C<sv> was corrected in 5.11.0 and between 5.11.0
+and 5.19.5 the functions were marked NULLOK. As an optimization the NULLOK code
+has now been removed, and the functions became non-NULL marked again, because
+core getter-type macros never pass NULL to these functions and would crash
+before ever passing NULL.
+
+The only way a NULL C<sv> can be passed to sv_2*v* functions is if XS code
+directly calls sv_2*v*. This is unlikely as XS code uses Sv*V* macros to get
+the underlying value out of the SV. One possible situation which leads to
+a NULL C<sv> being passed to sv_2*v* functions, is if XS code defines its own
+getter type Sv*V* macros, which check for NULL B<before> dereferencing and
+checking the SV's flags through public API Sv*OK* macros or directly using
+private API C<SvFLAGS>, and if C<sv> is NULL, then calling the sv_2*v functions
+with a NULL litteral or passing the C<sv> containing a NULL value.
 
 =back
 
diff --git a/proto.h b/proto.h
index 6b50e48..54a501d 100644 (file)
--- a/proto.h
+++ b/proto.h
@@ -3886,17 +3886,37 @@ PERL_CALLCONV IO*       Perl_sv_2io(pTHX_ SV *const sv)
 #define PERL_ARGS_ASSERT_SV_2IO        \
        assert(sv)
 
-/* PERL_CALLCONV IV    Perl_sv_2iv(pTHX_ SV *sv); */
-PERL_CALLCONV IV       Perl_sv_2iv_flags(pTHX_ SV *const sv, const I32 flags);
+/* PERL_CALLCONV IV    Perl_sv_2iv(pTHX_ SV *sv)
+                       __attribute__nonnull__(pTHX_1); */
+#define PERL_ARGS_ASSERT_SV_2IV        \
+       assert(sv)
+
+PERL_CALLCONV IV       Perl_sv_2iv_flags(pTHX_ SV *const sv, const I32 flags)
+                       __attribute__nonnull__(pTHX_1);
+#define PERL_ARGS_ASSERT_SV_2IV_FLAGS  \
+       assert(sv)
+
 PERL_CALLCONV SV*      Perl_sv_2mortal(pTHX_ SV *const sv);
 PERL_CALLCONV SV*      Perl_sv_2num(pTHX_ SV *const sv)
                        __attribute__nonnull__(pTHX_1);
 #define PERL_ARGS_ASSERT_SV_2NUM       \
        assert(sv)
 
-PERL_CALLCONV NV       Perl_sv_2nv_flags(pTHX_ SV *const sv, const I32 flags);
-/* PERL_CALLCONV char* Perl_sv_2pv(pTHX_ SV *sv, STRLEN *lp); */
-PERL_CALLCONV char*    Perl_sv_2pv_flags(pTHX_ SV *const sv, STRLEN *const lp, const I32 flags);
+PERL_CALLCONV NV       Perl_sv_2nv_flags(pTHX_ SV *const sv, const I32 flags)
+                       __attribute__nonnull__(pTHX_1);
+#define PERL_ARGS_ASSERT_SV_2NV_FLAGS  \
+       assert(sv)
+
+/* PERL_CALLCONV char* Perl_sv_2pv(pTHX_ SV *sv, STRLEN *lp)
+                       __attribute__nonnull__(pTHX_1); */
+#define PERL_ARGS_ASSERT_SV_2PV        \
+       assert(sv)
+
+PERL_CALLCONV char*    Perl_sv_2pv_flags(pTHX_ SV *const sv, STRLEN *const lp, const I32 flags)
+                       __attribute__nonnull__(pTHX_1);
+#define PERL_ARGS_ASSERT_SV_2PV_FLAGS  \
+       assert(sv)
+
 /* PERL_CALLCONV char* Perl_sv_2pv_nolen(pTHX_ SV* sv)
                        __attribute__warn_unused_result__
                        __attribute__nonnull__(pTHX_1); */
@@ -3925,8 +3945,16 @@ PERL_CALLCONV char*      Perl_sv_2pvutf8(pTHX_ SV *sv, STRLEN *const lp)
 #define PERL_ARGS_ASSERT_SV_2PVUTF8_NOLEN      \
        assert(sv)
 
-/* PERL_CALLCONV UV    Perl_sv_2uv(pTHX_ SV *sv); */
-PERL_CALLCONV UV       Perl_sv_2uv_flags(pTHX_ SV *const sv, const I32 flags);
+/* PERL_CALLCONV UV    Perl_sv_2uv(pTHX_ SV *sv)
+                       __attribute__nonnull__(pTHX_1); */
+#define PERL_ARGS_ASSERT_SV_2UV        \
+       assert(sv)
+
+PERL_CALLCONV UV       Perl_sv_2uv_flags(pTHX_ SV *const sv, const I32 flags)
+                       __attribute__nonnull__(pTHX_1);
+#define PERL_ARGS_ASSERT_SV_2UV_FLAGS  \
+       assert(sv)
+
 PERL_CALLCONV int      Perl_sv_backoff(pTHX_ SV *const sv)
                        __attribute__nonnull__(pTHX_1);
 #define PERL_ARGS_ASSERT_SV_BACKOFF    \
diff --git a/sv.c b/sv.c
index 9cfddc1..7507056 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -2300,8 +2300,7 @@ Perl_sv_2iv_flags(pTHX_ SV *const sv, const I32 flags)
 {
     dVAR;
 
-    if (!sv)
-       return 0;
+    PERL_ARGS_ASSERT_SV_2IV_FLAGS;
 
     assert (SvTYPE(sv) != SVt_PVAV && SvTYPE(sv) != SVt_PVHV
         && SvTYPE(sv) != SVt_PVFM);
@@ -2396,8 +2395,7 @@ Perl_sv_2uv_flags(pTHX_ SV *const sv, const I32 flags)
 {
     dVAR;
 
-    if (!sv)
-       return 0;
+    PERL_ARGS_ASSERT_SV_2UV_FLAGS;
 
     if (SvGMAGICAL(sv) && (flags & SV_GMAGIC))
        mg_get(sv);
@@ -2478,8 +2476,9 @@ NV
 Perl_sv_2nv_flags(pTHX_ SV *const sv, const I32 flags)
 {
     dVAR;
-    if (!sv)
-       return 0.0;
+
+    PERL_ARGS_ASSERT_SV_2NV_FLAGS;
+
     assert (SvTYPE(sv) != SVt_PVAV && SvTYPE(sv) != SVt_PVHV
         && SvTYPE(sv) != SVt_PVFM);
     if (SvGMAGICAL(sv) || SvVALID(sv) || isREGEXP(sv)) {
@@ -2782,11 +2781,8 @@ Perl_sv_2pv_flags(pTHX_ SV *const sv, STRLEN *const lp, const I32 flags)
     dVAR;
     char *s;
 
-    if (!sv) {
-       if (lp)
-           *lp = 0;
-       return (char *)"";
-    }
+    PERL_ARGS_ASSERT_SV_2PV_FLAGS;
+
     assert (SvTYPE(sv) != SVt_PVAV && SvTYPE(sv) != SVt_PVHV
         && SvTYPE(sv) != SVt_PVFM);
     if (SvGMAGICAL(sv) && (flags & SV_GMAGIC))