Last modified: 2010-10-13 01:53:39 UTC
Actually I'm not sure if I'm not wrong, because I thought something like this should cause a crash, so maybe it's ok, but... $pass = self::editSpacingCheck( $wgFlaggedRevsAutoconfirm['spacing'], $wgFlaggedRevsAutoconfirm['benchmarks'], $user ); # Make a key to store the results if ( !$pass ) { $wgMemc->set( $APSkipKey, 'true', 3600 * 24 * $spacing * ( $benchmarks - $needed - 1 ) ); return true; } else { $wgMemc->set( $sTestKey, 'true', 7 * 24 * 3600 ); } $spacing, $benchmarks and $needed seem to be local variables in self::editSpacingCheck(). As far as I can see there are no such variables in the current scope. Or are there?
BTW: $wgMemc->set( $APSkipKey, 'true', 3600 * 24 * $spacing * ( $benchmarks - $needed - 1 ) ); Isn't $benchmarks less than $needed?
This all looks like some fairly old code, so if it's broken, it's been broken for a while. That said, I agree that it looks like there might be some scope problems with $spacing, $benchmarks and $needed (at a minimum, there's an explode statement or something that needs a comment explaining where the variables come from) I'm leaving this assigned to myself for now, but will reassign when we're not in a release crunch.
Probably crept in when editSpacingCheck was made it's own function. This just seems to make should-be negative cache hits not hit. Fixing this now.
(In reply to comment #0) > Actually I'm not sure if I'm not wrong, because I thought something like this > should cause a crash, so maybe it's ok, but... > > $pass = self::editSpacingCheck( > $wgFlaggedRevsAutoconfirm['spacing'], > $wgFlaggedRevsAutoconfirm['benchmarks'], > $user > ); > # Make a key to store the results > if ( !$pass ) { > $wgMemc->set( $APSkipKey, 'true', > 3600 * 24 * $spacing * ( $benchmarks - $needed - 1 ) ); > return true; > } else { > $wgMemc->set( $sTestKey, 'true', 7 * 24 * 3600 ); > } > > > $spacing, $benchmarks and $needed seem to be local variables in > self::editSpacingCheck(). As far as I can see there are no such variables in > the current scope. Or are there? Fixed in r74710
(In reply to comment #1) > BTW: > $wgMemc->set( $APSkipKey, 'true', 3600 * 24 * $spacing * ( $benchmarks - > $needed - 1 ) ); > > Isn't $benchmarks less than $needed? Also inadvertently fixed in r74710. Those were obviously backwards.
