[Scummvm-git-logs] scummvm master -> 75bfee7d6b2e6c41651ce4d85c75b0a9b97c2d82
sev-
noreply at scummvm.org
Sat Jan 20 22:58:45 UTC 2024
This automated email contains information about 4 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
bc2942d73a BASE: Ensure folder path when file path set in command line
60b1731d47 BASE: Merge accessibility check and folder path extraction in reusable method
047942f5f2 BASE: Fix error in usage message for screenshot path
75bfee7d6b BASE: Add path option check for both readable and writeable
Commit: bc2942d73aea9c1ecd191a1f71c517bd8f3c4ea6
https://github.com/scummvm/scummvm/commit/bc2942d73aea9c1ecd191a1f71c517bd8f3c4ea6
Author: antoniou79 (a.antoniou79 at gmail.com)
Date: 2024-01-20T23:58:40+01:00
Commit Message:
BASE: Ensure folder path when file path set in command line
soundfont option is excluded for this, since that is expected to be a file path
Changed paths:
base/commandLine.cpp
diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index 6d50eb61294..8b0e2122c9b 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -678,6 +678,12 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
usage("Non-existent game path '%s'", option);
} else if (!path.isWritable()) {
usage("Non-writable screenshot path '%s'", option);
+ } else if (!path.isDirectory()
+ && path.getParent().isDirectory()
+ && path.getParent().isWritable()) {
+ // The path.getParent().isDirectory() check is redundant but included for clarifying
+ // the purpose of this case, which is to get the folder path part from a file path
+ settings["screenshotpath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
#endif
@@ -778,6 +784,12 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
usage("Non-existent game path '%s'", option);
} else if (!path.isReadable()) {
usage("Non-readable game path '%s'", option);
+ } else if (!path.isDirectory()
+ && path.getParent().isDirectory()
+ && path.getParent().isReadable()) {
+ // The path.getParent().isDirectory() check is redundant but included for clarifying
+ // the purpose of this case, which is to get the folder path part from a file path
+ settings["path"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
@@ -874,6 +886,12 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
usage("Non-existent saved games path '%s'", option);
} else if (!path.isWritable()) {
usage("Non-writable saved games path '%s'", option);
+ } else if (!path.isDirectory()
+ && path.getParent().isDirectory()
+ && path.getParent().isWritable()) {
+ // The path.getParent().isDirectory() check is redundant but included for clarifying
+ // the purpose of this case, which is to get the folder path part from a file path
+ settings["savepath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
@@ -883,16 +901,28 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
usage("Non-existent extra path '%s'", option);
} else if (!path.isReadable()) {
usage("Non-readable extra path '%s'", option);
+ } else if (!path.isDirectory()
+ && path.getParent().isDirectory()
+ && path.getParent().isReadable()) {
+ // The path.getParent().isDirectory() check is redundant but included for clarifying
+ // the purpose of this case, which is to get the folder path part from a file path
+ settings["extrapath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
DO_LONG_OPTION("iconspath")
- Common::FSNode path(option);
- if (!path.exists()) {
- usage("Non-existent icons path '%s'", option);
- } else if (!path.isReadable()) {
- usage("Non-readable icons path '%s'", option);
- }
+ Common::FSNode path(option);
+ if (!path.exists()) {
+ usage("Non-existent icons path '%s'", option);
+ } else if (!path.isReadable()) {
+ usage("Non-readable icons path '%s'", option);
+ } else if (!path.isDirectory()
+ && path.getParent().isDirectory()
+ && path.getParent().isReadable()) {
+ // The path.getParent().isDirectory() check is redundant but included for clarifying
+ // the purpose of this case, which is to get the folder path part from a file path
+ settings["iconspath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
+ }
END_OPTION
DO_LONG_OPTION("md5-path")
@@ -933,6 +963,12 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
usage("Non-existent theme path '%s'", option);
} else if (!path.isReadable()) {
usage("Non-readable theme path '%s'", option);
+ } else if (!path.isDirectory()
+ && path.getParent().isDirectory()
+ && path.getParent().isReadable()) {
+ // The path.getParent().isDirectory() check is redundant but included for clarifying
+ // the purpose of this case, which is to get the folder path part from a file path
+ settings["themepath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
Commit: 60b1731d4781a13758143cd650129c60d8314873
https://github.com/scummvm/scummvm/commit/60b1731d4781a13758143cd650129c60d8314873
Author: antoniou79 (a.antoniou79 at gmail.com)
Date: 2024-01-20T23:58:40+01:00
Commit Message:
BASE: Merge accessibility check and folder path extraction in reusable method
Changed paths:
base/commandLine.cpp
diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index 8b0e2122c9b..393797b5556 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -476,6 +476,39 @@ static Common::String createTemporaryTarget(const Common::String &engineId, cons
return domainName;
}
+/**
+ * A re-usable auxiliary method to ensure that the value given for a path command line option is
+ * a folder path, and meets the specified readability / writeability requirements.
+ * If the path given on command line is a file path, the method will use the parent folder path instead,
+ * updating the "settings" table and checking the readability / writeability requirements for that one.
+ * This method is to be used from within parseCommandLine() method.
+ * Note: The method assumes that path.exists() check was already done and is true.
+ *
+ * @param settings A reference to the settings map used by parseCommandLine()
+ * @param optionKeyStr The key string for updating the value for this path option on the settings map, if needed
+ * @param path The path node that was created from the command line value for this path option
+ * @param ensureWriteable A boolean flag that is set true if the path should be writeable, false otherwise
+ * @return true if given path was already a folder path or, if it was a file path, then a parent folder path
+ * was deduced from it, and the path (original or deduced respectively) meets the specified
+ * readability / writeability requirements.
+ */
+bool ensureAccessibleDirectoryForPathOption(Common::StringMap &settings, const Common::String optionKeyStr, const Common::FSNode &path, bool ensureWriteable) {
+ if (path.isDirectory()) {
+ if ((ensureWriteable && path.isWritable())
+ || (!ensureWriteable && path.isReadable())) {
+ return true;
+ }
+ } else if (path.getParent().isDirectory()
+ && ((ensureWriteable && path.getParent().isWritable())
+ || (!ensureWriteable && path.getParent().isReadable())) ) {
+ // The path.getParent().isDirectory() check is redundant but included for clarifying
+ // the purpose of this case, which is to get the folder path part from a file path
+ settings[optionKeyStr] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
+ return true;
+ }
+ return false;
+}
+
//
// Various macros used by the command line parser.
//
@@ -676,14 +709,8 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent game path '%s'", option);
- } else if (!path.isWritable()) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "screenshotpath", path, true)) {
usage("Non-writable screenshot path '%s'", option);
- } else if (!path.isDirectory()
- && path.getParent().isDirectory()
- && path.getParent().isWritable()) {
- // The path.getParent().isDirectory() check is redundant but included for clarifying
- // the purpose of this case, which is to get the folder path part from a file path
- settings["screenshotpath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
#endif
@@ -782,14 +809,8 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent game path '%s'", option);
- } else if (!path.isReadable()) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "path", path, false)) {
usage("Non-readable game path '%s'", option);
- } else if (!path.isDirectory()
- && path.getParent().isDirectory()
- && path.getParent().isReadable()) {
- // The path.getParent().isDirectory() check is redundant but included for clarifying
- // the purpose of this case, which is to get the folder path part from a file path
- settings["path"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
@@ -884,14 +905,8 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent saved games path '%s'", option);
- } else if (!path.isWritable()) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "savepath", path, true)) {
usage("Non-writable saved games path '%s'", option);
- } else if (!path.isDirectory()
- && path.getParent().isDirectory()
- && path.getParent().isWritable()) {
- // The path.getParent().isDirectory() check is redundant but included for clarifying
- // the purpose of this case, which is to get the folder path part from a file path
- settings["savepath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
@@ -899,14 +914,8 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent extra path '%s'", option);
- } else if (!path.isReadable()) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "extrapath", path, false)) {
usage("Non-readable extra path '%s'", option);
- } else if (!path.isDirectory()
- && path.getParent().isDirectory()
- && path.getParent().isReadable()) {
- // The path.getParent().isDirectory() check is redundant but included for clarifying
- // the purpose of this case, which is to get the folder path part from a file path
- settings["extrapath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
@@ -914,14 +923,8 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent icons path '%s'", option);
- } else if (!path.isReadable()) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "iconspath", path, false)) {
usage("Non-readable icons path '%s'", option);
- } else if (!path.isDirectory()
- && path.getParent().isDirectory()
- && path.getParent().isReadable()) {
- // The path.getParent().isDirectory() check is redundant but included for clarifying
- // the purpose of this case, which is to get the folder path part from a file path
- settings["iconspath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
@@ -961,14 +964,8 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent theme path '%s'", option);
- } else if (!path.isReadable()) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "themepath", path, false)) {
usage("Non-readable theme path '%s'", option);
- } else if (!path.isDirectory()
- && path.getParent().isDirectory()
- && path.getParent().isReadable()) {
- // The path.getParent().isDirectory() check is redundant but included for clarifying
- // the purpose of this case, which is to get the folder path part from a file path
- settings["themepath"] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
}
END_OPTION
Commit: 047942f5f217136f520b11292b521235307f6ab2
https://github.com/scummvm/scummvm/commit/047942f5f217136f520b11292b521235307f6ab2
Author: antoniou79 (a.antoniou79 at gmail.com)
Date: 2024-01-20T23:58:40+01:00
Commit Message:
BASE: Fix error in usage message for screenshot path
Changed paths:
base/commandLine.cpp
diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index 393797b5556..30bdb12ac87 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -708,7 +708,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
DO_LONG_OPTION("screenshotpath")
Common::FSNode path(option);
if (!path.exists()) {
- usage("Non-existent game path '%s'", option);
+ usage("Non-existent screenshot path '%s'", option);
} else if (!ensureAccessibleDirectoryForPathOption(settings, "screenshotpath", path, true)) {
usage("Non-writable screenshot path '%s'", option);
}
Commit: 75bfee7d6b2e6c41651ce4d85c75b0a9b97c2d82
https://github.com/scummvm/scummvm/commit/75bfee7d6b2e6c41651ce4d85c75b0a9b97c2d82
Author: antoniou79 (a.antoniou79 at gmail.com)
Date: 2024-01-20T23:58:40+01:00
Commit Message:
BASE: Add path option check for both readable and writeable
Also made the ensureAccessibleDirectoryForPathOption() method use a recursive call to reduce complexity
The following decisions are made for commandline path options accessibility checks:
"screenshotpath" option is required to be writeable (not checked for readable)
"path" option is required to be readable (not checked for writeable)
"savepath" option is required to be readable AND writeable
"extrapath" option is required to be readable (not checked for writeable)
"iconspath" option is required to be readable AND writeable
"themepath" option is required to be readable (not checked for writeable)
Changed paths:
base/commandLine.cpp
diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index 30bdb12ac87..aec21fd26e2 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -482,29 +482,36 @@ static Common::String createTemporaryTarget(const Common::String &engineId, cons
* If the path given on command line is a file path, the method will use the parent folder path instead,
* updating the "settings" table and checking the readability / writeability requirements for that one.
* This method is to be used from within parseCommandLine() method.
- * Note: The method assumes that path.exists() check was already done and is true.
+ * Note 1: The method assumes that path.exists() check was already done and is true.
+ * Note 2: The method will work with paths that are symbolic links to folders (isDirectory() returns true),
+ * but for symbolic links to files it will not deduce a valid folder path and will just return false.
*
- * @param settings A reference to the settings map used by parseCommandLine()
+ * @param settings A reference to the settings map used by parseCommandLine()
* @param optionKeyStr The key string for updating the value for this path option on the settings map, if needed
* @param path The path node that was created from the command line value for this path option
* @param ensureWriteable A boolean flag that is set true if the path should be writeable, false otherwise
+ * @param ensureReadable A boolean flag that is set true if the path should be readable, false otherwise
+ * @param acceptFile true if the command line option allows (tolerates) a file path to deduce the folder path from
* @return true if given path was already a folder path or, if it was a file path, then a parent folder path
* was deduced from it, and the path (original or deduced respectively) meets the specified
* readability / writeability requirements.
*/
-bool ensureAccessibleDirectoryForPathOption(Common::StringMap &settings, const Common::String optionKeyStr, const Common::FSNode &path, bool ensureWriteable) {
+bool ensureAccessibleDirectoryForPathOption(Common::StringMap &settings,
+ const Common::String optionKeyStr,
+ const Common::FSNode &path,
+ bool ensureWriteable,
+ bool ensureReadable,
+ bool acceptFile) {
if (path.isDirectory()) {
- if ((ensureWriteable && path.isWritable())
- || (!ensureWriteable && path.isReadable())) {
+ if ((!ensureWriteable || path.isWritable())
+ && (!ensureReadable || path.isReadable())
+ && (ensureWriteable || ensureReadable)) {
return true;
}
- } else if (path.getParent().isDirectory()
- && ((ensureWriteable && path.getParent().isWritable())
- || (!ensureWriteable && path.getParent().isReadable())) ) {
- // The path.getParent().isDirectory() check is redundant but included for clarifying
- // the purpose of this case, which is to get the folder path part from a file path
- settings[optionKeyStr] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
- return true;
+ } else if (acceptFile
+ && ensureAccessibleDirectoryForPathOption(settings, optionKeyStr, path.getParent(), ensureWriteable, ensureReadable, false)) {
+ settings[optionKeyStr] = path.getParent().getPath().toString(Common::Path::kNativeSeparator);
+ return true;
}
return false;
}
@@ -709,7 +716,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent screenshot path '%s'", option);
- } else if (!ensureAccessibleDirectoryForPathOption(settings, "screenshotpath", path, true)) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "screenshotpath", path, true, false, true)) {
usage("Non-writable screenshot path '%s'", option);
}
END_OPTION
@@ -809,7 +816,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent game path '%s'", option);
- } else if (!ensureAccessibleDirectoryForPathOption(settings, "path", path, false)) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "path", path, false, true, true)) {
usage("Non-readable game path '%s'", option);
}
END_OPTION
@@ -905,7 +912,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent saved games path '%s'", option);
- } else if (!ensureAccessibleDirectoryForPathOption(settings, "savepath", path, true)) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "savepath", path, true, true, true)) {
usage("Non-writable saved games path '%s'", option);
}
END_OPTION
@@ -914,7 +921,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent extra path '%s'", option);
- } else if (!ensureAccessibleDirectoryForPathOption(settings, "extrapath", path, false)) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "extrapath", path, false, true, true)) {
usage("Non-readable extra path '%s'", option);
}
END_OPTION
@@ -923,7 +930,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent icons path '%s'", option);
- } else if (!ensureAccessibleDirectoryForPathOption(settings, "iconspath", path, false)) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "iconspath", path, true, true, true)) {
usage("Non-readable icons path '%s'", option);
}
END_OPTION
@@ -964,7 +971,7 @@ Common::String parseCommandLine(Common::StringMap &settings, int argc, const cha
Common::FSNode path(option);
if (!path.exists()) {
usage("Non-existent theme path '%s'", option);
- } else if (!ensureAccessibleDirectoryForPathOption(settings, "themepath", path, false)) {
+ } else if (!ensureAccessibleDirectoryForPathOption(settings, "themepath", path, false, true, true)) {
usage("Non-readable theme path '%s'", option);
}
END_OPTION
More information about the Scummvm-git-logs
mailing list