From c41aa896101b2eaf12b2f65220f091ccb375d200 Mon Sep 17 00:00:00 2001
From: Ahmad Samir <a.samirh78@gmail.com>
Date: Thu, 10 Oct 2024 19:30:52 +0300
Subject: [PATCH] QTranslator: refactor loading translation files

QTranslator::load() used to:
- call find_translation(), which finds the file, ensures it's readable,
  and returns file name
- pass the file name to do_load() which would proceed to open the
  file

During the time between calling find_translation()/is_readable_file()
and calling do_load(), the underlying filesystem could have changed.

Solve the issue by making find_translation() call do_load() itself (as
suggested in the bug report). Also rename the method to
load_translation() and make it a member method so that it can call
do_load().

Remove the now redundant is_readable_file() static helper, since
QFile::open() in do_load() will do the necessary checks.

Note that there is a slight behavior change:
- with this commit we call do_load() on a variant of the translation
  file name, if that fails we call it again on the next variant ...etc
- before this, if the file was readable, the code called do_load() once,
  if it failed, it didn't try again.
This was suggested by Volker in code review as a good/welcome
side-effect.

Task-number: QTBUG-129777
Change-Id: Ie5fada60dd6267e67cda4272530db69a44f3ee55
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/kernel/qtranslator.cpp | 49 ++++++++++++------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/src/corelib/kernel/qtranslator.cpp b/src/corelib/kernel/qtranslator.cpp
index 4e02f2b589f..f3846e38c23 100644
--- a/src/corelib/kernel/qtranslator.cpp
+++ b/src/corelib/kernel/qtranslator.cpp
@@ -292,6 +292,8 @@ public:
     QString language;
     QString filePath;
 
+    bool load_translation(const QLocale &locale, const QString &filename, const QString &prefix,
+                          const QString &directory, const QString &suffix);
     bool do_load(const QString &filename, const QString &directory);
     bool do_load(const uchar *data, qsizetype len, const QString &directory);
     QString do_translate(const char *context, const char *sourceText, const char *comment,
@@ -596,21 +598,11 @@ bool QTranslatorPrivate::do_load(const QString &realname, const QString &directo
     return false;
 }
 
-Q_NEVER_INLINE
-static bool is_readable_file(const QString &name)
-{
-    const QFileInfo fi(name);
-    const bool isReadableFile = fi.isReadable() && fi.isFile();
-    qCDebug(lcTranslator) << "Testing file" << name << isReadableFile;
-
-    return isReadableFile;
-}
-
-static QString find_translation(const QLocale & locale,
-                                const QString & filename,
-                                const QString & prefix,
-                                const QString & directory,
-                                const QString & suffix)
+bool QTranslatorPrivate::load_translation(const QLocale &locale,
+                                          const QString &filename,
+                                          const QString &prefix,
+                                          const QString &directory,
+                                          const QString &suffix)
 {
     qCDebug(lcTranslator).noquote().nospace() << "Searching translation for "
                           << filename << prefix << locale << suffix
@@ -638,19 +630,21 @@ static QString find_translation(const QLocale & locale,
     const QStringList languages = locale.uiLanguages(QLocale::TagSeparator::Underscore);
     qCDebug(lcTranslator) << "Requested UI languages" << languages;
 
+    auto loadFile = [this, &realname, &directory] { return do_load(realname, directory); };
+
     for (const QString &localeName : languages) {
         QString loc = localeName;
         // First try this given name, then in lower-case form (if different):
         while (true) {
             // First, try with suffix:
             realname += loc + suffixOrDotQM;
-            if (is_readable_file(realname))
-                return realname;
+            if (loadFile())
+                return true;
 
             // Next, try without:
             realname.truncate(realNameBaseSize + loc.size());
-            if (is_readable_file(realname))
-                return realname;
+            if (loadFile())
+                return true;
             // Reset realname:
             realname.truncate(realNameBaseSize);
 
@@ -669,22 +663,18 @@ static QString find_translation(const QLocale & locale,
     if (!suffix.isNull()) {
         realname.replace(realNameBaseSizeFallbacks, prefix.size(), suffix);
         // realname == path + filename;
-        if (is_readable_file(realname))
-            return realname;
+        if (loadFile())
+            return true;
         realname.replace(realNameBaseSizeFallbacks, suffix.size(), prefix);
     }
 
     // realname == path + filename + prefix;
-    if (is_readable_file(realname))
-        return realname;
+    if (loadFile())
+        return true;
 
     realname.truncate(realNameBaseSizeFallbacks);
     // realname == path + filename;
-    if (is_readable_file(realname))
-        return realname;
-
-    realname.truncate(0);
-    return realname;
+    return loadFile();
 }
 
 /*!
@@ -738,8 +728,7 @@ bool QTranslator::load(const QLocale & locale,
 {
     Q_D(QTranslator);
     d->clear();
-    QString fname = find_translation(locale, filename, prefix, directory, suffix);
-    return !fname.isEmpty() && d->do_load(fname, directory);
+    return d->load_translation(locale, filename, prefix, directory, suffix);
 }
 
 /*!