r91409 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91408‎ | r91409 | r91410 >
Date:15:00, 4 July 2011
Author:ialex
Status:ok (Comments)
Tags:
Comment:
* Removed usage of error suppression operator in includes/db
* Changed an usage of $_REQUEST to $wgRequest
Modified paths:
  • /trunk/phase3/includes/db/Database.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseError.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseIbm_db2.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseMysql.php (modified) (history)
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabaseMysql.php
——@@ -95,7 +95,9 @@
9696 wfProfileOut("dbconnect-$server");
9797
9898 if ( $dbName != '' && $this->mConn !== false ) {
99 - $success = @/**/mysql_select_db( $dbName, $this->mConn );
 99+ wfSuppressWarnings();
 100+ $success = mysql_select_db( $dbName, $this->mConn );
 101+ wfRestoreWarnings();
100102 if ( !$success ) {
101103 $error = "Error selecting database $dbName on server {$this->mServer} " .
102104 "from client host " . wfHostname() . "\n";
——@@ -152,7 +154,10 @@
153155 if ( $res instanceof ResultWrapper ) {
154156 $res = $res->result;
155157 }
156 - if ( !@/**/mysql_free_result( $res ) ) {
 158+ wfSuppressWarnings();
 159+ $ok = mysql_free_result( $res );
 160+ wfRestoreWarnings();
 161+ if ( !$ok ) {
157162 throw new DBUnexpectedError( $this, "Unable to free MySQL result" );
158163 }
159164 }
——@@ -161,7 +166,9 @@
162167 if ( $res instanceof ResultWrapper ) {
163168 $res = $res->result;
164169 }
165 - @/**/$row = mysql_fetch_object( $res );
 170+ wfSuppressWarnings();
 171+ $row = mysql_fetch_object( $res );
 172+ wfRestoreWarnings();
166173 if( $this->lastErrno() ) {
167174 throw new DBUnexpectedError( $this, 'Error in fetchObject(): ' . htmlspecialchars( $this->lastError() ) );
168175 }
——@@ -172,7 +179,9 @@
173180 if ( $res instanceof ResultWrapper ) {
174181 $res = $res->result;
175182 }
176 - @/**/$row = mysql_fetch_array( $res );
 183+ wfSuppressWarnings();
 184+ $row = mysql_fetch_array( $res );
 185+ wfRestoreWarnings();
177186 if ( $this->lastErrno() ) {
178187 throw new DBUnexpectedError( $this, 'Error in fetchRow(): ' . htmlspecialchars( $this->lastError() ) );
179188 }
——@@ -183,7 +192,9 @@
184193 if ( $res instanceof ResultWrapper ) {
185194 $res = $res->result;
186195 }
187 - @/**/$n = mysql_num_rows( $res );
 196+ wfSuppressWarnings();
 197+ $n = mysql_num_rows( $res );
 198+ wfRestoreWarnings();
188199 if( $this->lastErrno() ) {
189200 throw new DBUnexpectedError( $this, 'Error in numRows(): ' . htmlspecialchars( $this->lastError() ) );
190201 }
Index: trunk/phase3/includes/db/DatabasePostgres.php
——@@ -263,7 +263,10 @@
264264 if ( $res instanceof ResultWrapper ) {
265265 $res = $res->result;
266266 }
267 - if ( !@pg_free_result( $res ) ) {
 267+ wfSuppressWarnings();
 268+ $ok = pg_free_result( $res );
 269+ wfRestoreWarnings();
 270+ if ( !$ok ) {
268271 throw new DBUnexpectedError( $this, "Unable to free Postgres result\n" );
269272 }
270273 }
——@@ -272,7 +275,9 @@
273276 if ( $res instanceof ResultWrapper ) {
274277 $res = $res->result;
275278 }
276 - @$row = pg_fetch_object( $res );
 279+ wfSuppressWarnings();
 280+ $row = pg_fetch_object( $res );
 281+ wfRestoreWarnings();
277282 # @todo FIXME: HACK HACK HACK HACK debug
278283
279284 # @todo hashar: not sure if the following test really trigger if the object
——@@ -287,7 +292,9 @@
288293 if ( $res instanceof ResultWrapper ) {
289294 $res = $res->result;
290295 }
291 - @$row = pg_fetch_array( $res );
 296+ wfSuppressWarnings();
 297+ $row = pg_fetch_array( $res );
 298+ wfRestoreWarnings();
292299 if( pg_last_error( $this->mConn ) ) {
293300 throw new DBUnexpectedError( $this, 'SQL error: ' . htmlspecialchars( pg_last_error( $this->mConn ) ) );
294301 }
——@@ -298,7 +305,9 @@
299306 if ( $res instanceof ResultWrapper ) {
300307 $res = $res->result;
301308 }
302 - @$n = pg_num_rows( $res );
 309+ wfSuppressWarnings();
 310+ $n = pg_num_rows( $res );
 311+ wfRestoreWarnings();
303312 if( pg_last_error( $this->mConn ) ) {
304313 throw new DBUnexpectedError( $this, 'SQL error: ' . htmlspecialchars( pg_last_error( $this->mConn ) ) );
305314 }
Index: trunk/phase3/includes/db/DatabaseError.php
——@@ -17,7 +17,7 @@
1818 * @param $error String A simple error message to be used for debugging
1919 */
2020 function __construct( DatabaseBase &$db, $error ) {
21 - $this->db =& $db;
 21+ $this->db = $db;
2222 parent::__construct( $error );
2323 }
2424
——@@ -178,13 +178,13 @@
179179 * @return string
180180 */
181181 function searchForm() {
182 - global $wgSitename, $wgServer;
 182+ global $wgSitename, $wgServer, $wgRequest;
183183
184184 $usegoogle = htmlspecialchars( $this->msg( 'dberr-usegoogle', 'You can try searching via Google in the meantime.' ) );
185185 $outofdate = htmlspecialchars( $this->msg( 'dberr-outofdate', 'Note that their indexes of our content may be out of date.' ) );
186186 $googlesearch = htmlspecialchars( $this->msg( 'searchbutton', 'Search' ) );
187187
188 - $search = htmlspecialchars( @$_REQUEST['search'] );
 188+ $search = htmlspecialchars( $wgRequest->getVal( 'search' ) );
189189
190190 $server = htmlspecialchars( $wgServer );
191191 $sitename = htmlspecialchars( $wgSitename );
Index: trunk/phase3/includes/db/DatabaseIbm_db2.php
——@@ -379,7 +379,9 @@
380380 * Opens a cataloged database connection, sets mConn
381381 */
382382 protected function openCataloged( $dbName, $user, $password ) {
383 - @$this->mConn = db2_pconnect( $dbName, $user, $password );
 383+ wfSuppressWarnings();
 384+ $this->mConn = db2_pconnect( $dbName, $user, $password );
 385+ wfRestoreWarnings();
384386 }
385387
386388 /**
——@@ -388,7 +390,9 @@
389391 protected function openUncataloged( $dbName, $user, $password, $server, $port )
390392 {
391393 $dsn = "DRIVER={IBM DB2 ODBC DRIVER};DATABASE=$dbName;CHARSET=UTF-8;HOSTNAME=$server;PORT=$port;PROTOCOL=TCPIP;UID=$user;PWD=$password;";
392 - @$this->mConn = db2_pconnect($dsn, "", "", array());
 394+ wfSuppressWarnings();
 395+ $this->mConn = db2_pconnect($dsn, "", "", array());
 396+ wfRestoreWarnings();
393397 }
394398
395399 /**
——@@ -501,7 +505,7 @@
502506 }
503507
504508 // If the table exists, there should be one of it
505 - @$row = $this->fetchRow( $res );
 509+ $row = $this->fetchRow( $res );
506510 $count = $row[0];
507511 if ( $count == '1' || $count == 1 ) {
508512 return true;
——@@ -523,7 +527,9 @@
524528 if ( $res instanceof ResultWrapper ) {
525529 $res = $res->result;
526530 }
527 - @$row = db2_fetch_object( $res );
 531+ wfSuppressWarnings();
 532+ $row = db2_fetch_object( $res );
 533+ wfRestoreWarnings();
528534 if( $this->lastErrno() ) {
529535 throw new DBUnexpectedError( $this, 'Error in fetchObject(): '
530536 . htmlspecialchars( $this->lastError() ) );
——@@ -544,7 +550,9 @@
545551 $res = $res->result;
546552 }
547553 if ( db2_num_rows( $res ) > 0) {
548 - @$row = db2_fetch_array( $res );
 554+ wfSuppressWarnings();
 555+ $row = db2_fetch_array( $res );
 556+ wfRestoreWarnings();
549557 if ( $this->lastErrno() ) {
550558 throw new DBUnexpectedError( $this, 'Error in fetchRow(): '
551559 . htmlspecialchars( $this->lastError() ) );
——@@ -1072,7 +1080,10 @@
10731081 if ( $res instanceof ResultWrapper ) {
10741082 $res = $res->result;
10751083 }
1076 - if ( !@db2_free_result( $res ) ) {
 1084+ wfSuppressWarnings();
 1085+ $ok = db2_free_result( $res );
 1086+ wfRestoreWarnings();
 1087+ if ( !$ok ) {
10771088 throw new DBUnexpectedError( $this, "Unable to free DB2 result\n" );
10781089 }
10791090 }
Index: trunk/phase3/includes/db/Database.php
——@@ -1877,9 +1877,9 @@
18781878 # the correct table.
18791879 $dbDetails = array_reverse( explode( '.', $name, 2 ) );
18801880 if ( isset( $dbDetails[1] ) ) {
1881 - @list( $table, $database ) = $dbDetails;
 1881+ list( $table, $database ) = $dbDetails;
18821882 } else {
1883 - @list( $table ) = $dbDetails;
 1883+ list( $table ) = $dbDetails;
18841884 }
18851885 $prefix = $this->mTablePrefix; # Default prefix
18861886

Comments

#Comment by 😂 (talk | contribs)   16:52, 4 July 2011

I think the $_REQUEST in here was because WebRequest might not be available at this stage. It's been awhile since I checked so that might not be strictly true anymore.

#Comment by Brion VIBBER (talk | contribs)   16:41, 5 July 2011

Seems to work; if I break my DB connection settings the error page search form correctly sees the 'search' qs param. Marking OK.

Status & tagging log