[Templates] bad XS bug [PATCH]

Chris Nandor template@pudge.net
Thu, 20 Feb 2003 16:38:19 -0500


I wrote:
>	FOREACH [0, 1];
>	        cols.$curcol = 1;
>	        curcol = curcol + 1;
>	Dumper.dump([cols, curcol]);
>	END;

>	$VAR1 = [
>	          [
>	            1
>	          ],
>	          1
>	        ];
>	$VAR1 = [
>	          [
>	            1,
>	            ${\$VAR1->[0][0]}
>	          ],
>	          2
>	        ];
>
>I expect that last reference to be "1", not a reference.  If I use
>Template::Stash instead of Template::Stash::XS, then I get the expected
>result.

Upon further review, this workaround "fixes" it:

[%
	USE Dumper;
	curcol  = 0;
	cols    = [];

	Dumper.dump([cols, curcol]);
	FOREACH [0, 1];
		temp = 1;
	        cols.$curcol = temp;
	        curcol = curcol + 1;
	Dumper.dump([cols, curcol]);
	END;
%]

Yes, just putting it in a temp value fixes the problem.  Ouch!  So armed
with this new information, I delved into Stash.xs and came up with this
patch:

--- xs/Stash.xs.orig    Thu Feb 20 16:29:13 2003
+++ xs/Stash.xs Thu Feb 20 16:29:17 2003
@@ -681,11 +681,12 @@
                    return &PL_sv_undef;
            }

+           newsv = newSVsv(value);
            if (looks_like_number(key_sv)) {
-               if (av_store(rootav, SvIV(key_sv), value))
-                   SvREFCNT_inc(value);
+               if (av_store(rootav, SvIV(key_sv), newsv))
+                   SvREFCNT_inc(newsv);
                else
-                   SvSETMAGIC(value);
+                   SvSETMAGIC(newsv);

                return value;
            }


I am not 100% sure that it is the right thing to do, but it mirrors what
happens immediately above it in case SVt_PVHV, and it seems to fix the
problem.  In Slash, we are going to stick with the temp-var workaround for
now, since we don't want to deal with a possibly incorrect patch, and many
other sites using our code might not have this patch anyway.  :)

Thanks to Jamie McCarthy who did most of the legwork identifying the
problem and Chip Salzenberg for helping me sanity-check the patch.

-- 
Chris Nandor                      pudge@pobox.com    http://pudge.net/
Open Source Development Network    pudge@osdn.com     http://osdn.com/