From d8533815de8b3467521e33666f4bb29f4af46445 Mon Sep 17 00:00:00 2001
From: Tobias Hunger <tobias.hunger@qt.io>
Date: Wed, 19 Apr 2017 14:47:48 +0200
Subject: [PATCH] Environment: Simplify code a bit

Move duplication check into class and do not duplicate that.

Change-Id: I6fa34fb2e39d47432e87e959d1e5f44d21287bad
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>
---
 src/libs/utils/environment.cpp | 50 +++++++++++++++++-----------------
 src/libs/utils/environment.h   |  3 +-
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/src/libs/utils/environment.cpp b/src/libs/utils/environment.cpp
index 6beae6d6157..d7ea3935648 100644
--- a/src/libs/utils/environment.cpp
+++ b/src/libs/utils/environment.cpp
@@ -23,9 +23,10 @@
 **
 ****************************************************************************/
 
-#include "algorithm.h"
 #include "environment.h"
 
+#include "algorithm.h"
+
 #include <QDir>
 #include <QProcessEnvironment>
 #include <QSet>
@@ -231,19 +232,22 @@ void Environment::clear()
     m_values.clear();
 }
 
-FileName Environment::searchInDirectory(const QStringList &execs, QString directory) const
+FileName Environment::searchInDirectory(const QStringList &execs, QString directory,
+                                        QSet<QString> &alreadyChecked) const
 {
     const QChar slash = '/';
-    if (directory.isEmpty())
-        return FileName();
     // Avoid turing / into // on windows which triggers windows to check
     // for network drives!
-    if (!directory.endsWith(slash))
-        directory += slash;
+    const QString dir = directory.endsWith(slash) ? directory : directory + slash;
 
-    foreach (const QString &exec, execs) {
-        QFileInfo fi(directory + exec);
-        if (fi.exists() && fi.isFile() && fi.isExecutable())
+    if (directory.isEmpty() || alreadyChecked.contains(dir))
+        return {};
+
+    alreadyChecked.insert(dir);
+
+    for (const QString &exec : execs) {
+        QFileInfo fi(dir + exec);
+        if (fi.isFile() && fi.isExecutable())
             return FileName::fromString(fi.absoluteFilePath());
     }
     return FileName();
@@ -286,36 +290,32 @@ FileName Environment::searchInPath(const QString &executable,
     if (executable.isEmpty())
         return FileName();
 
-    QString exec = QDir::cleanPath(expandVariables(executable));
-    QFileInfo fi(exec);
+    const QString exec = QDir::cleanPath(expandVariables(executable));
+    const QFileInfo fi(exec);
 
-    QStringList execs = appendExeExtensions(exec);
+    const QStringList execs = appendExeExtensions(exec);
 
     if (fi.isAbsolute()) {
-        foreach (const QString &path, execs)
-            if (QFile::exists(path))
+        for (const QString &path : execs) {
+            QFileInfo pfi = QFileInfo(path);
+            if (pfi.isFile() && pfi.isExecutable())
                 return FileName::fromString(path);
+        }
         return FileName::fromString(exec);
     }
 
     QSet<QString> alreadyChecked;
-    foreach (const QString &dir, additionalDirs) {
-        if (alreadyChecked.contains(dir))
-            continue;
-        alreadyChecked.insert(dir);
-        FileName tmp = searchInDirectory(execs, dir);
+    for (const QString &dir : additionalDirs) {
+        FileName tmp = searchInDirectory(execs, dir, alreadyChecked);
         if (!tmp.isEmpty() && (!func || func(tmp.toString())))
             return tmp;
     }
 
-    if (executable.indexOf('/') != -1)
+    if (executable.contains('/'))
         return FileName();
 
-    foreach (const QString &p, path()) {
-        if (alreadyChecked.contains(p))
-            continue;
-        alreadyChecked.insert(p);
-        FileName tmp = searchInDirectory(execs, QDir::fromNativeSeparators(p));
+    for (const QString &p : path()) {
+        FileName tmp = searchInDirectory(execs, QDir::fromNativeSeparators(p), alreadyChecked);
         if (!tmp.isEmpty() && (!func || func(tmp.toString())))
             return tmp;
     }
diff --git a/src/libs/utils/environment.h b/src/libs/utils/environment.h
index 2ce135ed85a..1dbcd856c04 100644
--- a/src/libs/utils/environment.h
+++ b/src/libs/utils/environment.h
@@ -118,7 +118,8 @@ public:
     bool operator==(const Environment &other) const;
 
 private:
-    FileName searchInDirectory(const QStringList &execs, QString directory) const;
+    FileName searchInDirectory(const QStringList &execs, QString directory,
+                               QSet<QString> &alreadyChecked) const;
     QMap<QString, QString> m_values;
     OsType m_osType;
 };