Extend SvREFCNT* to work on any perl variable type #12935
Comments
From @jmdhCreated by @jmdhThis is a bug report for perl from dom@earth.li, ----------------------------------------------------------------- "I recently discovered that Devel::Peek's SvREFCNT, SvREFCNT_inc .... " "\%m" is a reference constructor, so SvREFCNT() ends up #!/usr/bin/perl From the Dump you can see that %m (the PVHV) has a refcount of Devel::Peek::SvREFCNT(\%m) should be equivalent to my $tmp = \%m; which unsurprisingly reports a refcount of 1 for $tmp. Nothing changes The proposed patch solves this difficulty by always passing the Patch attached for review. Perl Info |
From @jmdh0001-Extend-SvREFCNT-to-work-on-any-perl-variable-type.patchFrom 68ee83ef1867e121aeb890f3d4f326126b2495f7 Mon Sep 17 00:00:00 2001
From: Robert Stone <talby@trap.mtview.ca.us>
Date: Thu, 30 Nov 2006 18:31:54 -0800
Subject: [PATCH] Extend SvREFCNT* to work on any perl variable type
This could break code in cases where users expect an array of one member
passed to these functions to operate on that only member of the array, but
for all other data types (and arrays of any other length) these functions
currently croak. Possibly a new family of functions is warranted to
maintain reverse compatibility in the single element array case in the
existing functions, but it is not my place to make that judgement.
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=401132
---
ext/Devel-Peek/Peek.xs | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/ext/Devel-Peek/Peek.xs b/ext/Devel-Peek/Peek.xs
index e97c979..1cc0fd3 100644
--- a/ext/Devel-Peek/Peek.xs
+++ b/ext/Devel-Peek/Peek.xs
@@ -389,15 +389,22 @@ PPCODE:
I32
SvREFCNT(sv)
SV * sv
+PROTOTYPE: \[$@%&*]
+PPCODE:
+{
+ RETVAL = SvREFCNT(SvRV(sv)) - 1; // -1 because our ref doesn't count
+ PUSHi(RETVAL);
+}
# PPCODE needed since otherwise sv_2mortal is inserted that will kill the value.
SV *
SvREFCNT_inc(sv)
SV * sv
+PROTOTYPE: \[$@%&*]
PPCODE:
{
- RETVAL = SvREFCNT_inc(sv);
+ RETVAL = SvREFCNT_inc(SvRV(sv));
PUSHs(RETVAL);
}
@@ -406,9 +413,10 @@ PPCODE:
void
SvREFCNT_dec(sv)
SV * sv
+PROTOTYPE: \[$@%&*]
PPCODE:
{
- SvREFCNT_dec(sv);
+ SvREFCNT_dec(SvRV(sv));
PUSHs(sv);
}
--
1.7.10.4
|
From @tonycozOn Sun, Apr 28, 2013 at 02:39:25PM -0700, Dominic Hargreaves wrote:
Or we could just deprecate/remove them. Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @nwc10On Mon, Apr 29, 2013 at 09:02:50AM +1000, Tony Cook wrote:
Yes. It's always going to be possible for people to use (or write) code But I'm really not convinced that we should be offering documented functions Nicholas Clark |
From @khwilliamsonOn 05/13/2013 09:03 AM, Nicholas Clark wrote:
+1 |
From @bulk88Nicholas Clark wrote:
The refcount checkers from Perl lang are useful to prove in a test that |
From @nwc10On Mon, May 13, 2013 at 01:36:06PM -0400, bulk88 wrote:
Agree, in that I could see that the perl core is reading reference counts in (That's not an argument that everything must change. Just a thought that Nicholas Clark |
From @HugmeirOn Sun, Apr 28, 2013 at 6:39 PM, Dominic Hargreaves <
If you really really need to mess with the refcounts, you might as well use &Internals::SvREFCNT(\@etc, 10); Enjoy your rope! |
From @tonycozOn Mon May 13 11:06:50 2013, nicholas wrote:
So should we be removing (or deprecating) the SvREFCNT_*() modifiers Or nuke them all, since they're available (if deliberately undocumented) Tony |
From @tonycozOn Sun Jun 23 18:04:00 2013, tonyc wrote:
Here's an alternative patch that: - removes the modifier functions - modifies the SvREFCNT() XS function to accept any type of object - adds a very basic test Note that the original patch leaked the SV SvREFCNT() returned. For debate, I'll apply it in a few days unless there's some Tony |
From @tonycozOn Thu Jul 11 17:28:56 2013, tonyc wrote:
This time with the patch. Thanks, khw. Tony |
From @tonycoz0001-perl-117793-remove-dangerous-functions-and-improve-S.patchFrom 5eb402dc3f63c00e3839efd02f8bd4ebe64be4d2 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Fri, 12 Jul 2013 10:14:57 +1000
Subject: [PATCH] [perl #117793] remove dangerous functions and improve
SvREFCNT()
This allows Devel::Peek::SvREFCNT() to work on any variable, not just
scalars, but has a chance of breaking backward compatibility.
Also changes the type of SvREFCNT() to UV to match the type returned by
the underlying macro
---
ext/Devel-Peek/Peek.pm | 5 ++---
ext/Devel-Peek/Peek.xs | 29 ++++++-----------------------
ext/Devel-Peek/t/Peek.t | 7 +++++++
3 files changed, 15 insertions(+), 26 deletions(-)
diff --git a/ext/Devel-Peek/Peek.pm b/ext/Devel-Peek/Peek.pm
index 2dacb54..f46864c 100644
--- a/ext/Devel-Peek/Peek.pm
+++ b/ext/Devel-Peek/Peek.pm
@@ -13,7 +13,7 @@ require XSLoader;
@ISA = qw(Exporter);
@EXPORT = qw(Dump mstat DeadCode DumpArray DumpWithOP DumpProg
fill_mstats mstats_fillhash mstats2hash runops_debug debug_flags);
-@EXPORT_OK = qw(SvREFCNT SvREFCNT_inc SvREFCNT_dec CvGV);
+@EXPORT_OK = qw(SvREFCNT CvGV);
%EXPORT_TAGS = ('ALL' => [@EXPORT, @EXPORT_OK]);
XSLoader::load();
@@ -98,8 +98,7 @@ Devel::Peek supplies a C<Dump()> function which can dump a raw Perl
datatype, and C<mstat("marker")> function to report on memory usage
(if perl is compiled with corresponding option). The function
DeadCode() provides statistics on the data "frozen" into inactive
-C<CV>. Devel::Peek also supplies C<SvREFCNT()>, C<SvREFCNT_inc()>, and
-C<SvREFCNT_dec()> which can query, increment, and decrement reference
+C<CV>. Devel::Peek also supplies C<SvREFCNT()> which can query reference
counts on SVs. This document will take a passive, and safe, approach
to data debugging and for that it will describe only the C<Dump()>
function.
diff --git a/ext/Devel-Peek/Peek.xs b/ext/Devel-Peek/Peek.xs
index e97c979..a8d79f3 100644
--- a/ext/Devel-Peek/Peek.xs
+++ b/ext/Devel-Peek/Peek.xs
@@ -386,31 +386,14 @@ PPCODE:
op_dump(PL_main_root);
}
-I32
+U32
SvREFCNT(sv)
SV * sv
-
-# PPCODE needed since otherwise sv_2mortal is inserted that will kill the value.
-
-SV *
-SvREFCNT_inc(sv)
-SV * sv
-PPCODE:
-{
- RETVAL = SvREFCNT_inc(sv);
- PUSHs(RETVAL);
-}
-
-# PPCODE needed since by default it is void
-
-void
-SvREFCNT_dec(sv)
-SV * sv
-PPCODE:
-{
- SvREFCNT_dec(sv);
- PUSHs(sv);
-}
+PROTOTYPE: \[$@%&*]
+CODE:
+ RETVAL = SvREFCNT(SvRV(sv)) - 1; /* -1 because our ref doesn't count */
+OUTPUT:
+ RETVAL
SV *
DeadCode()
diff --git a/ext/Devel-Peek/t/Peek.t b/ext/Devel-Peek/t/Peek.t
index 5019fb1..79a3fd7 100644
--- a/ext/Devel-Peek/t/Peek.t
+++ b/ext/Devel-Peek/t/Peek.t
@@ -1080,4 +1080,11 @@ do_test('UTF-8 in a regular expression',
SAVED_COPY = 0x0)?
');
+{ # perl #117793: Extend SvREFCNT* to work on any perl variable type
+ my %hash;
+ my $base_count = Devel::Peek::SvREFCNT(%hash);
+ my $ref = \%hash;
+ is(Devel::Peek::SvREFCNT(%hash), $base_count + 1, "SvREFCNT on non-scalar");
+}
+
done_testing();
--
1.7.10.4
|
From @tonycozOn Thu Jul 11 17:28:56 2013, tonyc wrote:
There was no response at all, everyone loves it! Applied as da1929e. Tony |
|
@tonycoz - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#117793 (status was 'resolved')
Searchable as RT117793$
The text was updated successfully, but these errors were encountered: