QString-based interface for interpret_tilde

server/stdinhand.cpp

@@ -33,6 +33,7 @@
 #include "rand.h"
 #include "registry.h"
 #include "section_file.h"
+#include "shared.h"
 #include "support.h" // fc__attribute, bool type, etc.
 #include "timing.h"
 
@@ -1134,44 +1135,39 @@ static bool read_init_script_real(struct connection *caller,
                                   int read_recursion)
 {
   FILE *script_file;
-  const char extension[] = ".serv";
-  char serv_filename[strlen(extension) + qstrlen(script_filename) + 2];
-  char tilde_filename[4096];
-  QString real_filename;
+  const auto extension = QLatin1String(".serv");
 
   // check recursion depth
   if (read_recursion > GAME_MAX_READ_RECURSION) {
     qCritical("Error: recursive calls to read!");
     return false;
   }
 
-  // abuse real_filename to find if we already have a .serv extension
-  real_filename = QString(script_filename);
-  if (!real_filename.endsWith(extension)) {
-    fc_snprintf(serv_filename, sizeof(serv_filename), "%s%s",
-                script_filename, extension);
-  } else {
-    sz_strlcpy(serv_filename, script_filename);
+  // Find if we already have a .serv extension
+  QString serv_filename = script_filename;
+  if (!serv_filename.endsWith(extension)) {
+    serv_filename += extension;
   }
 
+  QString tilde_filename;
   if (is_restricted(caller) && !from_cmdline) {
     if (!is_safe_filename(serv_filename)) {
       cmd_reply(CMD_READ_SCRIPT, caller, C_FAIL,
                 _("Name \"%s\" disallowed for security reasons."),
-                serv_filename);
+                qUtf8Printable(serv_filename));
       return false;
     }
-    sz_strlcpy(tilde_filename, serv_filename);
+    tilde_filename = serv_filename;
   } else {
-    interpret_tilde(tilde_filename, sizeof(tilde_filename), serv_filename);
+    tilde_filename = interpret_tilde(serv_filename);
   }
 
-  real_filename = fileinfoname(get_data_dirs(), tilde_filename);
+  auto real_filename = fileinfoname(get_data_dirs(), tilde_filename);
   if (real_filename.isEmpty()) {
     if (is_restricted(caller) && !from_cmdline) {
       cmd_reply(CMD_READ_SCRIPT, caller, C_FAIL,
                 _("No command script found by the name \"%s\"."),
-                serv_filename);
+                qUtf8Printable(serv_filename));
       return false;
     }
     // File is outside data directories
@@ -1231,13 +1227,13 @@ QVector<QString> *get_init_script_choices()
  */
 static void write_init_script(char *script_filename)
 {
-  char real_filename[1024], buf[256];
+  char buf[256];
   FILE *script_file;
 
-  interpret_tilde(real_filename, sizeof(real_filename), script_filename);
+  const auto real_filename = interpret_tilde(script_filename);
 
   if (QFile::exists(real_filename)
-      && (script_file = fc_fopen(real_filename, "w"))) {
+      && (script_file = fc_fopen(qUtf8Printable(real_filename), "w"))) {
     fprintf(script_file, "#FREECIV SERVER COMMAND FILE, version %s\n",
             freeciv21_version());
     fputs(
@@ -1289,7 +1285,8 @@ static void write_init_script(char *script_filename)
     fclose(script_file);
 
   } else {
-    qCritical(_("Could not write script file '%s'."), real_filename);
+    qCritical(_("Could not write script file '%s'."),
+              qUtf8Printable((real_filename)));
   }
 }
 
@@ -5020,8 +5017,8 @@ static bool lua_command(struct connection *caller, char *arg, bool check,
                         int read_recursion)
 {
   FILE *script_file;
-  const char extension[] = ".lua", *real_filename = nullptr;
-  char luafile[4096], tilde_filename[4096];
+  const auto extension = QLatin1String(".lua");
+  QString luafile, tilde_filename, real_filename;
   char *luaarg = nullptr;
   QStringList tokens;
   int ind;
@@ -5096,36 +5093,32 @@ static bool lua_command(struct connection *caller, char *arg, bool check,
     }
     // Fall through.
   case LUA_FILE:
-    // Abuse real_filename to find if we already have a .lua extension.
-    real_filename =
-        luaarg + qstrlen(luaarg) - MIN(strlen(extension), qstrlen(luaarg));
-    if (strcmp(real_filename, extension) != 0) {
-      fc_snprintf(luafile, sizeof(luafile), "%s%s", luaarg, extension);
-    } else {
-      sz_strlcpy(luafile, luaarg);
+    // find if we already have a .lua extension.
+    luafile = luaarg;
+    if (!luafile.endsWith(extension)) {
+      luafile += extension;
     }
 
     if (is_restricted(caller)) {
       if (!is_safe_filename(luafile)) {
         cmd_reply(
             CMD_LUA, caller, C_FAIL,
             _("Freeciv21 script '%s' disallowed for security reasons."),
-            luafile);
+            qUtf8Printable(luafile));
         return false;
         ;
       }
-      sz_strlcpy(tilde_filename, luafile);
+      tilde_filename = luafile;
     } else {
-      interpret_tilde(tilde_filename, sizeof(tilde_filename), luafile);
+      tilde_filename = interpret_tilde(luafile);
     }
 
-    real_filename =
-        qUtf8Printable(fileinfoname(get_data_dirs(), tilde_filename));
-    if (!real_filename) {
+    real_filename = fileinfoname(get_data_dirs(), tilde_filename);
+    if (real_filename.isEmpty()) {
       if (is_restricted(caller)) {
         cmd_reply(CMD_LUA, caller, C_FAIL,
                   _("No Freeciv21 script found by the name '%s'."),
-                  tilde_filename);
+                  qUtf8Printable(tilde_filename));
         return false;
       }
       // File is outside data directories
@@ -5147,31 +5140,36 @@ static bool lua_command(struct connection *caller, char *arg, bool check,
     break;
   case LUA_FILE:
     cmd_reply(CMD_LUA, caller, C_COMMENT,
-              _("Loading Freeciv21 script file '%s'."), real_filename);
+              _("Loading Freeciv21 script file '%s'."),
+              qUtf8Printable(real_filename));
 
     if (QFile::exists(real_filename)
-        && (script_file = fc_fopen(real_filename, "r"))) {
-      ret = script_server_do_file(caller, real_filename);
+        && (script_file = fc_fopen(qUtf8Printable(real_filename), "r"))) {
+      ret = script_server_do_file(caller, qUtf8Printable(real_filename));
       fclose(script_file);
       return ret;
     } else {
       cmd_reply(CMD_LUA, caller, C_FAIL,
-                _("Cannot read Freeciv21 script '%s'."), real_filename);
+                _("Cannot read Freeciv21 script '%s'."),
+                qUtf8Printable(real_filename));
       return false;
     }
     break;
   case LUA_UNSAFE_FILE:
     cmd_reply(CMD_LUA, caller, C_COMMENT,
-              _("Loading Freeciv21 script file '%s'."), real_filename);
+              _("Loading Freeciv21 script file '%s'."),
+              qUtf8Printable(real_filename));
 
     if (QFile::exists(real_filename)
-        && (script_file = fc_fopen(real_filename, "r"))) {
+        && (script_file = fc_fopen(qUtf8Printable(real_filename), "r"))) {
       fclose(script_file);
-      ret = script_server_unsafe_do_file(caller, real_filename);
+      ret = script_server_unsafe_do_file(caller,
+                                         qUtf8Printable(real_filename));
       return ret;
     } else {
       cmd_reply(CMD_LUA, caller, C_FAIL,
-                _("Cannot read Freeciv21 script '%s'."), real_filename);
+                _("Cannot read Freeciv21 script '%s'."),
+                qUtf8Printable(real_filename));
       return false;
     }
     break;

utility/registry_ini.cpp

@@ -568,9 +568,7 @@ struct section_file *secfile_load_section(const QString &filename,
                                           const QString &section,
                                           bool allow_duplicates)
 {
-  char real_filename[1024];
-
-  interpret_tilde(real_filename, sizeof(real_filename), filename);
+  const auto real_filename = interpret_tilde(filename);
   return secfile_from_input_file(inf_from_file(real_filename, datafilename),
                                  filename, section, allow_duplicates);
 }
@@ -609,7 +607,6 @@ static bool is_legal_table_entry_name(char c, bool num)
  */
 bool secfile_save(const struct section_file *secfile, QString filename)
 {
-  char real_filename[1024];
   char pentry_name[128];
   const char *col_entry_name;
   const struct entry_list_link *ent_iter, *save_iter, *col_iter;
@@ -622,13 +619,13 @@ bool secfile_save(const struct section_file *secfile, QString filename)
     filename = secfile->name;
   }
 
-  interpret_tilde(real_filename, sizeof(real_filename), filename);
+  auto real_filename = interpret_tilde(filename);
   auto fs = std::make_unique<KFilterDev>(real_filename);
   fs->open(QIODevice::WriteOnly);
 
   if (!fs->isOpen()) {
     SECFILE_LOG(secfile, nullptr, _("Could not open %s for writing"),
-                real_filename);
+                qUtf8Printable((real_filename)));
     return false;
   }
 
@@ -766,7 +763,8 @@ bool secfile_save(const struct section_file *secfile, QString filename)
                     "a less efficient non-tabular format will be used.\n"
                     "To avoid this make sure all rows of a table are\n"
                     "filled out with an entry for every column.",
-                    real_filename, section_name(psection), expect);
+                    qUtf8Printable(real_filename), section_name(psection),
+                    expect);
                 fc_assert_ret_val(fs->write("\n") > 0, false);
               }
               fc_assert_ret_val(fs->write("}\n") > 0, false);
@@ -835,7 +833,8 @@ bool secfile_save(const struct section_file *secfile, QString filename)
 
   if (fs->error() != 0) {
     SECFILE_LOG(secfile, nullptr, "Error before closing %s: %s",
-                real_filename, qUtf8Printable(fs->errorString()));
+                qUtf8Printable(real_filename),
+                qUtf8Printable(fs->errorString()));
     return false;
   }
 

utility/shared.cpp

@@ -1123,62 +1123,28 @@ void free_multicast_group()
 }
 
 /**
-   Interpret ~/ in filename as home dir
-   New path is returned in buf of size buf_size
-
-   This may fail if the path is too long.  It is better to use
-   interpret_tilde_alloc.
- */
-void interpret_tilde(char *buf, size_t buf_size, const QString &filename)
-{
-  if (filename.startsWith(QLatin1String("~/"))) {
-    fc_snprintf(buf, buf_size, "%s/%s", qUtf8Printable(QDir::homePath()),
-                qUtf8Printable(filename.right(filename.length() - 2)));
-  } else if (filename == QLatin1String("~")) {
-    qstrncpy(buf, qUtf8Printable(QDir::homePath()), buf_size);
-  } else {
-    qstrncpy(buf, qUtf8Printable(filename), buf_size);
-  }
-}
-
-/**
-   Interpret ~/ in filename as home dir
-
-   The new path is returned in buf, as a newly allocated buffer.  The new
-   path will always be allocated and written, even if there is no ~ present.
+ * Interpret ~ in filename as home dir
  */
-char *interpret_tilde_alloc(const char *filename)
+QString interpret_tilde(const QString &filename)
 {
-  if (filename[0] == '~' && filename[1] == '/') {
-    QString home = QDir::homePath();
-    size_t sz;
-    char *buf;
-
-    filename += 2; /* Skip past "~/" */
-    sz = home.length() + qstrlen(filename) + 2;
-    buf = static_cast<char *>(fc_malloc(sz));
-    fc_snprintf(buf, sz, "%s/%s", qUtf8Printable(home), filename);
-    return buf;
-  } else if (filename[0] == '~' && filename[1] == '\0') {
-    return fc_strdup(qUtf8Printable(QDir::homePath()));
+  if (filename == QLatin1String("~")) {
+    return QDir::homePath();
+  } else if (filename.startsWith(QLatin1String("~/"))) {
+    return QDir::homePath() + filename.midRef(1);
   } else {
-    return fc_strdup(filename);
+    return filename;
   }
 }
 
 /**
    If the directory "pathname" does not exist, recursively create all
    directories until it does.
  */
-bool make_dir(const char *pathname)
+bool make_dir(const QString &pathname)
 {
-  auto *path = interpret_tilde_alloc(pathname);
-  auto str = QString::fromUtf8(path);
   // We can always create a directory with an empty name -- it's the current
   // folder.
-  auto r = str.isEmpty() || QDir().mkpath(str);
-  delete[] path;
-  return r;
+  return pathname.isEmpty() || QDir().mkpath(interpret_tilde(pathname));
 }
 
 /**

utility/shared.h

@@ -179,10 +179,10 @@ enum m_pre_result match_prefix_full(m_pre_accessor_fn_t accessor_fn,
 
 char *get_multicast_group(bool ipv6_preferred);
 void free_multicast_group();
-void interpret_tilde(char *buf, size_t buf_size, const QString &filename);
-char *interpret_tilde_alloc(const char *filename);
 
-bool make_dir(const char *pathname);
+QString interpret_tilde(const QString &filename);
+
+bool make_dir(const QString &pathname);
 
 char scanin(char **buf, char *delimiters, char *dest, int size);
 

utility/tests/test_paths.cpp

@@ -9,9 +9,20 @@ class test_paths : public QObject {
   Q_OBJECT
 
 private slots:
+  void interpret_tilde();
   void is_safe_filename();
+  void make_dir();
 };
 
+/**
+ * Tests \ref ::interpret_tilde
+ */
+void test_paths::interpret_tilde()
+{
+  QCOMPARE(::interpret_tilde(QLatin1String("~")), QDir::homePath());
+  QCOMPARE(::interpret_tilde(QLatin1String("test")), QLatin1String("test"));
+}
+
 /**
  * Tests \ref ::is_safe_filename
  */
@@ -23,5 +34,15 @@ void test_paths::is_safe_filename()
   QCOMPARE(::is_safe_filename(QLatin1String("..")), false);
 }
 
+/**
+ * Tests \ref ::make_dir
+ */
+void test_paths::make_dir()
+{
+  // The main difference between make_dir and QDir()::mkpath is that make_dir
+  // ignores empty paths. We don't check that QDir works as expected.
+  QVERIFY(::make_dir(QLatin1String("")));
+}
+
 QTEST_MAIN(test_paths)
 #include "test_paths.moc"