develooper Front page | perl.perl5.porters | Postings from August 2001

Re: [ID 20010730.010] FETCH called twice with -T

Thread Previous | Thread Next
From:
Abhijit Menon-Sen
Date:
August 10, 2001 17:48
Subject:
Re: [ID 20010730.010] FETCH called twice with -T
Message ID:
20010811061819.P526@lustre.dyn.wiw.org
At 2001-07-30 20:05:27, abigail@foad.org wrote:
> 
> That is, the first time a tied scalar is fetched, it is fetched twice.

This happens when the return value from FETCH is tainted. The FETCH adds
taint magic to the SV, which confuses mg_get() into walking the linked
list of magic twice.

This patch makes mg_get() a lot smarter about handling changes to the
list while it's running. It fixes the problem and passes all tests, but
I'd appreciate reviews, since it's not an entirely trivial change.

Incidentally:

1. Does the SV ever actually get upgraded, as the old comments suggest?
   When? (I can find nothing in the core which does this.)

2. Does anything depend in particular upon the order in which the magic
   vtbl functions are called? I first tried was changing sv_magic() to
   add to the end of the list instead, but that broke a lot of things
   (which I think I'll go back and investigate.)

- ams

--- current/mg.c~	Fri Aug 10 13:42:26 2001
+++ current/mg.c	Sat Aug 11 05:39:47 2001
@@ -93,34 +93,48 @@
 int
 Perl_mg_get(pTHX_ SV *sv)
 {
-    I32 mgs_ix;
-    MAGIC* mg;
-    MAGIC** mgp;
-    int mgp_valid = 0;
+    int new = 0;
+    MAGIC *newmg, *head, *cur, *mg;
+    I32 mgs_ix = SSNEW(sizeof(MGS));
 
-    mgs_ix = SSNEW(sizeof(MGS));
     save_magic(mgs_ix, sv);
 
-    mgp = &SvMAGIC(sv);
-    while ((mg = *mgp) != 0) {
-	MGVTBL* vtbl = mg->mg_virtual;
+    /* We must call svt_get(sv, mg) for each valid entry in the linked
+       list of magic. svt_get() may delete the current entry, add new
+       magic to the head of the list, or upgrade the SV. AMS 20010810 */
+
+    newmg = cur = head = mg = SvMAGIC(sv);
+    while (mg) {
+	MGVTBL *vtbl = mg->mg_virtual;
+
 	if (!(mg->mg_flags & MGf_GSKIP) && vtbl && vtbl->svt_get) {
 	    CALL_FPTR(vtbl->svt_get)(aTHX_ sv, mg);
-	    /* Ignore this magic if it's been deleted */
-	    if ((mg == (mgp_valid ? *mgp : SvMAGIC(sv))) &&
-		  (mg->mg_flags & MGf_GSKIP))
-		(SSPTR(mgs_ix, MGS*))->mgs_flags = 0;
+	    /* Don't restore the flags for this entry if it was deleted. */
+	    if (mg->mg_flags & MGf_GSKIP)
+		(SSPTR(mgs_ix, MGS *))->mgs_flags = 0;
+	}
+
+	mg = mg->mg_moremagic;
+
+	if (new) {
+	    /* Have we finished with the new entries we saw? Start again
+	       where we left off (unless there are more new entries). */
+	    if (mg == head) {
+		new  = 0;
+		mg   = cur;
+		head = newmg;
+	    }
 	}
-	/* Advance to next magic (complicated by possible deletion) */
-	if (mg == (mgp_valid ? *mgp : SvMAGIC(sv))) {
-	    mgp = &mg->mg_moremagic;
-	    mgp_valid = 1;
+
+	/* Were any new entries added? */
+	if (!new && (newmg = SvMAGIC(sv)) != head) {
+	    new = 1;
+	    cur = mg;
+	    mg  = newmg;
 	}
-	else
-	    mgp = &SvMAGIC(sv);	/* Re-establish pointer after sv_upgrade */
     }
 
-    restore_magic(aTHXo_ INT2PTR(void*, (IV)mgs_ix));
+    restore_magic(aTHXo_ INT2PTR(void *, (IV)mgs_ix));
     return 0;
 }
 

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About