r76185 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r76184‎ | r76185 | r76186 >
Date:11:30, 6 November 2010
Author:hashar
Status:ok (Comments)
Tags:
Comment:
reinsert some statements removed by r74745

You removed a free() statement in filerepo/LocalRepo.php
In Wiki.php the explicit $ret = null was welcomed
Modified paths:
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/includes/filerepo/LocalRepo.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/filerepo/LocalRepo.php
——@@ -167,6 +167,7 @@
168168 foreach ( $res as $row ) {
169169 $result[] = $this->newFileFromRow( $row );
170170 }
 171+ $res->free();
171172
172173 return $result;
173174 }
Index: trunk/phase3/includes/Wiki.php
——@@ -108,6 +108,7 @@
109109 if( $wgRequest->getVal( 'printable' ) === 'yes' ) {
110110 $wgOut->setPrintable();
111111 }
 112+ $ret = null;
112113 $curid = $wgRequest->getInt( 'curid' );
113114 if( $curid ) {
114115 // URLs like this are generated by RC, because rc_title isn't always accurate

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r74745Assignment in loop conditions suck...reedy23:11, 13 October 2010

Comments

#Comment by Nikerabbit (talk | contribs)   11:35, 6 November 2010

The function ends few lines below. Why is the free() necessary here? Most of the time it is not needed and variables get cleaned up after they go out of scope.

#Comment by Hashar (talk | contribs)   11:45, 6 November 2010

I have added it back since I was not sure why it was called. It might useless though :)

#Comment by Reedy (talk | contribs)   16:27, 6 November 2010

The $ret = null; is redundant

It's unconditionally assigned to in all paths through the if clause

#Comment by Nikerabbit (talk | contribs)   17:21, 6 November 2010

It might be useful to be there to avoid warnings when the code is refactored.

#Comment by Hashar (talk | contribs)   17:52, 6 November 2010

That's why I have put it back. It might avoid errors to.

Status & tagging log