r114126 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r114125‎ | r114126 | r114127 >
Date:11:14, 19 March 2012
Author:qchris
Status:ok (Comments)
Tags:
Comment:
Sanitizing already existing use of PHP's assert
Modified paths:
  • /trunk/phase3/includes/diff/DairikiDiff.php (modified) (history)
  • /trunk/phase3/includes/parser/Preprocessor_DOM.php (modified) (history)
  • /trunk/phase3/includes/parser/Preprocessor_Hash.php (modified) (history)
  • /trunk/phase3/includes/parser/Preprocessor_HipHop.hphp (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/diff/DairikiDiff.php
@@ -185,8 +185,8 @@
186186 $edits = array();
187187 $xi = $yi = 0;
188188 while ( $xi < $n_from || $yi < $n_to ) {
189 - assert( $yi < $n_to || $this->xchanged[$xi] );
190 - assert( $xi < $n_from || $this->ychanged[$yi] );
 189+ assert( '$yi < $n_to || $this->xchanged[$xi]' );
 190+ assert( '$xi < $n_from || $this->ychanged[$yi]' );
191191
192192 // Skip matching "snake".
193193 $copy = array();
@@ -374,14 +374,14 @@
375375 while ( list( , $y ) = each( $matches ) ) {
376376 if ( empty( $this->in_seq[$y] ) ) {
377377 $k = $this->_lcs_pos( $y );
378 - assert( $k > 0 );
 378+ assert( '$k > 0' );
379379 $ymids[$k] = $ymids[$k -1];
380380 break;
381381 }
382382 }
383383 while ( list ( , $y ) = each( $matches ) ) {
384384 if ( $y > $this->seq[$k -1] ) {
385 - assert( $y < $this->seq[$k] );
 385+ assert( '$y < $this->seq[$k]' );
386386 // Optimization: this is a common case:
387387 // next match is just replacing previous match.
388388 $this->in_seq[$this->seq[$k]] = false;
@@ -389,7 +389,7 @@
390390 $this->in_seq[$y] = 1;
391391 } elseif ( empty( $this->in_seq[$y] ) ) {
392392 $k = $this->_lcs_pos( $y );
393 - assert( $k > 0 );
 393+ assert( '$k > 0' );
394394 $ymids[$k] = $ymids[$k -1];
395395 }
396396 }
@@ -430,7 +430,7 @@
431431 }
432432 }
433433
434 - assert( $ypos != $this->seq[$end] );
 434+ assert( '$ypos != $this->seq[$end]' );
435435
436436 $this->in_seq[$this->seq[$end]] = false;
437437 $this->seq[$end] = $ypos;
@@ -814,8 +814,8 @@
815815 $mapped_from_lines, $mapped_to_lines ) {
816816 wfProfileIn( __METHOD__ );
817817
818 - assert( sizeof( $from_lines ) == sizeof( $mapped_from_lines ) );
819 - assert( sizeof( $to_lines ) == sizeof( $mapped_to_lines ) );
 818+ assert( 'sizeof( $from_lines ) == sizeof( $mapped_from_lines )' );
 819+ assert( 'sizeof( $to_lines ) == sizeof( $mapped_to_lines )' );
820820
821821 parent::__construct( $mapped_from_lines, $mapped_to_lines );
822822
@@ -1205,7 +1205,7 @@
12061206 $this->_flushLine( $tag );
12071207 $word = substr( $word, 1 );
12081208 }
1209 - assert( !strstr( $word, "\n" ) );
 1209+ assert( '!strstr( $word, "\n" )' );
12101210 $this->_group .= $word;
12111211 }
12121212 }
Index: trunk/phase3/includes/parser/Preprocessor_Hash.php
@@ -428,7 +428,7 @@
429429 } elseif ( $found == 'line-end' ) {
430430 $piece = $stack->top;
431431 // A heading must be open, otherwise \n wouldn't have been in the search list
432 - assert( $piece->open == "\n" );
 432+ assert( '$piece->open == "\n"' );
433433 $part = $piece->getCurrentPart();
434434 // Search back through the input to see if it has a proper close
435435 // Do this using the reversed string since the other solutions (end anchor, etc.) are inefficient
Index: trunk/phase3/includes/parser/Preprocessor_HipHop.hphp
@@ -444,7 +444,7 @@
445445 } elseif ( $found === 'line-end' ) {
446446 $piece = $stack->getTop();
447447 // A heading must be open, otherwise \n wouldn't have been in the search list
448 - assert( $piece->open === "\n" );
 448+ assert( '$piece->open === "\n"' );
449449 $part = $piece->getCurrentPart();
450450 // Search back through the input to see if it has a proper close
451451 // Do this using the reversed string since the other solutions (end anchor, etc.) are inefficient
Index: trunk/phase3/includes/parser/Preprocessor_DOM.php
@@ -479,7 +479,7 @@
480480 } elseif ( $found == 'line-end' ) {
481481 $piece = $stack->top;
482482 // A heading must be open, otherwise \n wouldn't have been in the search list
483 - assert( $piece->open == "\n" );
 483+ assert( '$piece->open == "\n"' );
484484 $part = $piece->getCurrentPart();
485485 // Search back through the input to see if it has a proper close
486486 // Do this using the reversed string since the other solutions (end anchor, etc.) are inefficient

Follow-up revisions

RevisionCommit summaryAuthorDate
r114217Follow up to r114126: Being more conservative for HipHop compilerqchris23:36, 19 March 2012

Comments

#Comment by Nikerabbit (talk | contribs)   11:40, 19 March 2012

BTW, does HipHop support assert?

#Comment by QChris (WMF) (talk | contribs)   23:44, 19 March 2012

Yes, absolutely :D HipHop does support assert.

However, as HipHop is taylored for production servers, it comes with different defaults: Assertion checking (and warning) is turned off per default, as it should be for production servers.

Everything works nicely on the HipHop Interpreter.

But I just realized, that when not interpreting the file and turning on assertion checking and passing as the condition as string and using the vanilla distribution, the executable chokes. So, to be more defensive, I reverted Preprocessor_HipHop.hphp in r114217 to avoid that the assert gets in the way.

Status & tagging log