Reorganise the functions doing wfProfileOut and returning on all if branches.

Makes life easier for static analysis, since they don't need to
handle if the end of a function where a wfProfileOut was not called
was reachable or not.

It is recommended to review this change ignoring whitespaces
(specially for includes/parser/Tidy.php)

Also documented the rationale for the elseif chain in UploadBase::detectVirus()

Change-Id: Ic4f65937fa9e6f926d8fcfd670e3b0e99e06eefc

diff --git a/includes/Article.php b/includes/Article.php
index 8eb58a9..10cbac7 100644 (file)
--- a/includes/Article.php
+++ b/includes/Article.php
@@ -280,15 +280,13 @@ class Article extends Page {
                                $message = $this->getContext()->getUser()->isLoggedIn() ? 'noarticletext' : 'noarticletextanon';
                                $content = new MessageContent( $message, null, 'parsemag' );
                        }
-                       wfProfileOut( __METHOD__ );
-
-                       return $content;
                } else {
                        $this->fetchContentObject();
-                       wfProfileOut( __METHOD__ );
-
-                       return $this->mContentObject;
+                       $content = $this->mContentObject;
                }
+
+               wfProfileOut( __METHOD__ );
+               return $content;
        }
 
        /**

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 410fa3a..f3c0237 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -1650,12 +1650,7 @@ class EditPage {
                        $doEditStatus = $this->mArticle->doEditContent( $content, $this->summary, $flags,
                                                                                                                        false, null, $this->contentFormat );
 
-               if ( $doEditStatus->isOK() ) {
-                               $result['redirect'] = $content->isRedirect();
-                       $this->updateWatchlist();
-                       wfProfileOut( __METHOD__ );
-                       return $status;
-               } else {
+               if ( !$doEditStatus->isOK() ) {
                        // Failure from doEdit()
                        // Show the edit conflict page for certain recognized errors from doEdit(),
                        // but don't show it for errors from extension hooks
@@ -1670,6 +1665,11 @@ class EditPage {
                        wfProfileOut( __METHOD__ );
                        return $doEditStatus;
                }
+
+               $result['redirect'] = $content->isRedirect();
+               $this->updateWatchlist();
+               wfProfileOut( __METHOD__ );
+               return $status;
        }
 
        /**
@@ -1761,10 +1761,10 @@ class EditPage {
                        $editContent = $result;
                        wfProfileOut( __METHOD__ );
                        return true;
-               } else {
-                       wfProfileOut( __METHOD__ );
-                       return false;
                }
+
+               wfProfileOut( __METHOD__ );
+               return false;
        }
 
        /**

diff --git a/includes/Linker.php b/includes/Linker.php
index 5c41b42..2f6ee22 100644 (file)
--- a/includes/Linker.php
+++ b/includes/Linker.php
@@ -947,10 +947,10 @@ class Linker {
                        return '<a href="' . htmlspecialchars( $href ) . '" class="new" title="' .
                                htmlspecialchars( $title->getPrefixedText(), ENT_QUOTES ) . '">' .
                                $encLabel . '</a>';
-               } else {
-                       wfProfileOut( __METHOD__ );
-                       return self::linkKnown( $title, $encLabel, array(), wfCgiToArray( $query ) );
                }
+
+               wfProfileOut( __METHOD__ );
+               return self::linkKnown( $title, $encLabel, array(), wfCgiToArray( $query ) );
        }
 
        /**

diff --git a/includes/db/DatabaseMysql.php b/includes/db/DatabaseMysql.php
index 8708608..04c22f1 100644 (file)
--- a/includes/db/DatabaseMysql.php
+++ b/includes/db/DatabaseMysql.php
@@ -597,10 +597,9 @@ class DatabaseMysql extends DatabaseBase {
                if ( $res && $row = $this->fetchRow( $res ) ) {
                        wfProfileOut( $fname );
                        return $row[0];
-               } else {
-                       wfProfileOut( $fname );
-                       return false;
                }
+               wfProfileOut( $fname );
+               return false;
        }
 
        /**

diff --git a/includes/parser/Tidy.php b/includes/parser/Tidy.php
index 5cc1b0f..dd7e965 100644 (file)
--- a/includes/parser/Tidy.php
+++ b/includes/parser/Tidy.php
@@ -260,24 +260,24 @@ class MWTidy {
 
                        wfProfileOut( __METHOD__ );
                        return $tidy->errorBuffer;
+               }
+
+               $tidy->cleanRepair();
+               $retval = $tidy->getStatus();
+               if ( $retval == 2 ) {
+                       // 2 is magic number for fatal error
+                       // http://www.php.net/manual/en/function.tidy-get-status.php
+                       $cleansource = null;
                } else {
-                       $tidy->cleanRepair();
-                       $retval = $tidy->getStatus();
-                       if ( $retval == 2 ) {
-                               // 2 is magic number for fatal error
-                               // http://www.php.net/manual/en/function.tidy-get-status.php
-                               $cleansource = null;
-                       } else {
-                               $cleansource = tidy_get_output( $tidy );
-                               if ( $wgDebugTidy && $retval > 0 ) {
-                                       $cleansource .= "<!--\nTidy reports:\n" .
-                                               str_replace( '-->', '--&gt;', $tidy->errorBuffer ) .
-                                               "\n-->";
-                               }
+                       $cleansource = tidy_get_output( $tidy );
+                       if ( $wgDebugTidy && $retval > 0 ) {
+                               $cleansource .= "<!--\nTidy reports:\n" .
+                                       str_replace( '-->', '--&gt;', $tidy->errorBuffer ) .
+                                       "\n-->";
                        }
-
-                       wfProfileOut( __METHOD__ );
-                       return $cleansource;
                }
+
+               wfProfileOut( __METHOD__ );
+               return $cleansource;
        }
 }

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 48ea584..b5c65e5 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -1234,27 +1234,22 @@ abstract class UploadBase {
                        }
                }
 
+               /* NB: AV_NO_VIRUS is 0 but AV_SCAN_FAILED is false,
+                * so we need the strict equalities === and thus can't use a switch here
+                */
                if ( $mappedCode === AV_SCAN_FAILED ) {
                        # scan failed (code was mapped to false by $exitCodeMap)
                        wfDebug( __METHOD__ . ": failed to scan $file (code $exitCode).\n" );
 
-                       if ( $wgAntivirusRequired ) {
-                               wfProfileOut( __METHOD__ );
-                               return wfMessage( 'virus-scanfailed', array( $exitCode ) )->text();
-                       } else {
-                               wfProfileOut( __METHOD__ );
-                               return null;
-                       }
+                       $output = $wgAntivirusRequired ? wfMessage( 'virus-scanfailed', array( $exitCode ) )->text() : null;
                } elseif ( $mappedCode === AV_SCAN_ABORTED ) {
                        # scan failed because filetype is unknown (probably imune)
                        wfDebug( __METHOD__ . ": unsupported file type $file (code $exitCode).\n" );
-                       wfProfileOut( __METHOD__ );
-                       return null;
+                       $output = null;
                } elseif ( $mappedCode === AV_NO_VIRUS ) {
                        # no virus found
                        wfDebug( __METHOD__ . ": file passed virus scan.\n" );
-                       wfProfileOut( __METHOD__ );
-                       return false;
+                       $output = false;
                } else {
                        $output = trim( $output );
 
@@ -1270,9 +1265,10 @@ abstract class UploadBase {
                        }
 
                        wfDebug( __METHOD__ . ": FOUND VIRUS! scanner feedback: $output \n" );
-                       wfProfileOut( __METHOD__ );
-                       return $output;
                }
+
+               wfProfileOut( __METHOD__ );
+               return $output;
        }
 
        /**