From 7686a2e2fce57c615b10c84094d3a81084cdb5ef Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Thu, 4 Oct 2018 20:56:33 +0000 Subject: [PATCH 01/18] lib/cmdline.c: Move `rm_cmd_read_paths_from_stdin()' Move it closer to `rm_cmd_set_paths()'. --- lib/cmdline.c | 51 ++++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index f457e7e1..0725f528 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -360,31 +360,6 @@ static GLogLevelFlags VERBOSITY_TO_LOG_LEVEL[] = {[0] = G_LOG_LEVEL_CRITICAL, [3] = G_LOG_LEVEL_MESSAGE | G_LOG_LEVEL_INFO, [4] = G_LOG_LEVEL_DEBUG}; -static bool rm_cmd_read_paths_from_stdin(RmSession *session, bool is_prefd, - bool null_separated) { - char delim = null_separated ? 0 : '\n'; - - size_t buf_len = PATH_MAX; - char *path_buf = malloc(buf_len * sizeof(char)); - - bool all_paths_read = true; - - int path_len; - - /* Still read all paths on errors, so the user knows all paths that failed */ - while((path_len = getdelim(&path_buf, &buf_len, delim, stdin)) >= 0) { - if(path_len > 0) { - /* replace returned delimiter with null */ - if (path_buf[path_len - 1] == delim) { - path_buf[path_len - 1] = 0; - } - all_paths_read &= rm_cfg_prepend_path(session->cfg, path_buf, is_prefd); - } - } - - free(path_buf); - return all_paths_read; -} static bool rm_cmd_parse_output_pair(RmSession *session, const char *pair, GError **error) { @@ -1170,6 +1145,32 @@ static bool rm_cmd_set_cmdline(RmCfg *cfg, int argc, char **argv) { return true; } +static bool rm_cmd_read_paths_from_stdin(RmSession *session, bool is_prefd, + bool null_separated) { + char delim = null_separated ? 0 : '\n'; + + size_t buf_len = PATH_MAX; + char *path_buf = malloc(buf_len * sizeof(char)); + + bool all_paths_read = true; + + int path_len; + + /* Still read all paths on errors, so the user knows all paths that failed */ + while((path_len = getdelim(&path_buf, &buf_len, delim, stdin)) >= 0) { + if(path_len > 0) { + /* replace returned delimiter with null */ + if(path_buf[path_len - 1] == delim) { + path_buf[path_len - 1] = 0; + } + all_paths_read &= rm_cfg_prepend_path(session->cfg, path_buf, is_prefd); + } + } + + free(path_buf); + return all_paths_read; +} + static bool rm_cmd_set_paths(RmSession *session, char **paths) { bool is_prefd = false; bool all_paths_valid = true; From 0e181c9186c425aa72df63b8f7d51ba94acef9fb Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Tue, 16 Oct 2018 07:39:02 +0000 Subject: [PATCH 02/18] lib/cmdline.c: Simplify size in call to `malloc()' --- lib/cmdline.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 0725f528..d16d5256 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1149,8 +1149,8 @@ static bool rm_cmd_read_paths_from_stdin(RmSession *session, bool is_prefd, bool null_separated) { char delim = null_separated ? 0 : '\n'; - size_t buf_len = PATH_MAX; - char *path_buf = malloc(buf_len * sizeof(char)); + size_t buf_len; + char *path_buf = malloc(buf_len = PATH_MAX); bool all_paths_read = true; From c33f727a1286e9223df3b498a2be1c182429add9 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Sat, 29 Sep 2018 17:12:22 +0000 Subject: [PATCH 03/18] lib/cmdline.c: Replace `RmSession' with `RmCfg' in `rm_cmd_set_paths*()' A pointer to an `RmSession' object was being passed around, but only a pointer to its associated `RmCfg' object is needed. --- lib/cmdline.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index d16d5256..9c33562b 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1145,8 +1145,12 @@ static bool rm_cmd_set_cmdline(RmCfg *cfg, int argc, char **argv) { return true; } -static bool rm_cmd_read_paths_from_stdin(RmSession *session, bool is_prefd, - bool null_separated) { +static INLINE +bool rm_cmd_set_paths_from_stdin( + RmCfg *const cfg, + const bool is_prefd, + const bool null_separated +) { char delim = null_separated ? 0 : '\n'; size_t buf_len; @@ -1163,7 +1167,7 @@ static bool rm_cmd_read_paths_from_stdin(RmSession *session, bool is_prefd, if(path_buf[path_len - 1] == delim) { path_buf[path_len - 1] = 0; } - all_paths_read &= rm_cfg_prepend_path(session->cfg, path_buf, is_prefd); + all_paths_read &= rm_cfg_prepend_path(cfg, path_buf, is_prefd); } } @@ -1171,13 +1175,13 @@ static bool rm_cmd_read_paths_from_stdin(RmSession *session, bool is_prefd, return all_paths_read; } -static bool rm_cmd_set_paths(RmSession *session, char **paths) { +static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { + g_assert(cfg); + bool is_prefd = false; bool all_paths_valid = true; bool stdin_paths_preferred = false; - RmCfg *cfg = session->cfg; - /* Check the directory to be valid */ for(int i = 0; paths && paths[i]; ++i) { if(strcmp(paths[i], "-") == 0) { @@ -1196,13 +1200,14 @@ static bool rm_cmd_set_paths(RmSession *session, char **paths) { if(cfg->read_stdin || cfg->read_stdin0) { /* option '-' means read paths from stdin */ - all_paths_valid &= - rm_cmd_read_paths_from_stdin(session, stdin_paths_preferred, cfg->read_stdin0); + all_paths_valid &= rm_cmd_set_paths_from_stdin( + cfg, stdin_paths_preferred, cfg->read_stdin0 + ); } if(g_slist_length(cfg->paths) == 0 && all_paths_valid) { /* Still no path set? - use `pwd` */ - rm_cfg_prepend_path(session->cfg, cfg->iwd, is_prefd); + rm_cfg_prepend_path(cfg, cfg->iwd, is_prefd); } /* Only return success if everything is fine. */ @@ -1467,7 +1472,7 @@ bool rm_cmd_parse_args(int argc, char **argv, RmSession *session) { goto cleanup; } - if(!rm_cmd_set_paths(session, paths)) { + if(!rm_cmd_set_paths(cfg, paths)) { error = g_error_new(RM_ERROR_QUARK, 0, _("Not all given paths are valid. Aborting")); goto cleanup; } From 88e0573d59edc0d0345360e5baa7188db38f3685 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Tue, 16 Oct 2018 04:14:12 +0000 Subject: [PATCH 04/18] lib/cmdline.c: Abstract out struct `rm_cmd_set_paths_vars' Variables that need to be shared across `rm_cmd_set_paths*()' may be placed in an instance of this struct. --- lib/cmdline.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 9c33562b..3973a35c 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1145,12 +1145,20 @@ static bool rm_cmd_set_cmdline(RmCfg *cfg, int argc, char **argv) { return true; } +typedef struct rm_cmd_set_paths_vars { + RmCfg *const cfg; + bool stdin_paths_preferred; +} rm_cmd_set_paths_vars; + static INLINE -bool rm_cmd_set_paths_from_stdin( - RmCfg *const cfg, - const bool is_prefd, - const bool null_separated -) { +bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { + g_assert(v); + g_assert(v->cfg); + + RmCfg *const cfg = v->cfg; + const bool is_prefd = v->stdin_paths_preferred; + const bool null_separated = cfg->read_stdin0; + char delim = null_separated ? 0 : '\n'; size_t buf_len; @@ -1180,14 +1188,16 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { bool is_prefd = false; bool all_paths_valid = true; - bool stdin_paths_preferred = false; + rm_cmd_set_paths_vars v = { + .cfg = cfg, + }; /* Check the directory to be valid */ for(int i = 0; paths && paths[i]; ++i) { if(strcmp(paths[i], "-") == 0) { cfg->read_stdin = true; /* remember whether to treat stdin paths as preferred paths */ - stdin_paths_preferred = is_prefd; + v.stdin_paths_preferred = is_prefd; } else if(strcmp(paths[i], "//") == 0) { /* the '//' separator separates non-preferred paths from preferred */ is_prefd = !is_prefd; @@ -1200,9 +1210,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { if(cfg->read_stdin || cfg->read_stdin0) { /* option '-' means read paths from stdin */ - all_paths_valid &= rm_cmd_set_paths_from_stdin( - cfg, stdin_paths_preferred, cfg->read_stdin0 - ); + all_paths_valid &= rm_cmd_set_paths_from_stdin(&v); } if(g_slist_length(cfg->paths) == 0 && all_paths_valid) { From 74b35742ceb7a97e1f2dd54ae5d5a8489cda71ac Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Mon, 15 Oct 2018 04:53:46 +0000 Subject: [PATCH 05/18] lib/cmdline.c: Add error handling to `rm_cmd_set_paths_from_stdin()' Before, it was just ignoring failures in `malloc()' and `getdelim()'. Also, one of the error messages in `lib/treemerge.c' has been set to match the error message introduced here. --- lib/cmdline.c | 29 ++++++++++++++++++++--------- lib/treemerge.c | 2 +- po/de.po | 14 +++++++++++--- po/es.po | 14 +++++++++++--- po/fr.po | 14 +++++++++++--- po/rmlint.pot | 16 ++++++++++++---- 6 files changed, 66 insertions(+), 23 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 3973a35c..56bb81a5 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1148,6 +1148,7 @@ static bool rm_cmd_set_cmdline(RmCfg *cfg, int argc, char **argv) { typedef struct rm_cmd_set_paths_vars { RmCfg *const cfg; bool stdin_paths_preferred; + bool all_paths_valid; } rm_cmd_set_paths_vars; static INLINE @@ -1163,8 +1164,10 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { size_t buf_len; char *path_buf = malloc(buf_len = PATH_MAX); - - bool all_paths_read = true; + if(!path_buf) { + rm_log_perror(_("Failed to allocate memory")); + return false; + } int path_len; @@ -1175,21 +1178,26 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { if(path_buf[path_len - 1] == delim) { path_buf[path_len - 1] = 0; } - all_paths_read &= rm_cfg_prepend_path(cfg, path_buf, is_prefd); + v->all_paths_valid &= rm_cfg_prepend_path(cfg, path_buf, is_prefd); } } + if(ferror(stdin)) { + rm_log_perror(_("Failed to read stdin")) + return false; + } + free(path_buf); - return all_paths_read; + return true; } static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_assert(cfg); bool is_prefd = false; - bool all_paths_valid = true; rm_cmd_set_paths_vars v = { .cfg = cfg, + .all_paths_valid = true }; /* Check the directory to be valid */ @@ -1202,7 +1210,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { /* the '//' separator separates non-preferred paths from preferred */ is_prefd = !is_prefd; } else { - all_paths_valid &= rm_cfg_prepend_path(cfg, paths[i], is_prefd); + v.all_paths_valid &= rm_cfg_prepend_path(cfg, paths[i], is_prefd); } } @@ -1210,16 +1218,19 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { if(cfg->read_stdin || cfg->read_stdin0) { /* option '-' means read paths from stdin */ - all_paths_valid &= rm_cmd_set_paths_from_stdin(&v); + if(!rm_cmd_set_paths_from_stdin(&v)) { + rm_log_error_line(_("Could not process path arguments")); + return false; + } } - if(g_slist_length(cfg->paths) == 0 && all_paths_valid) { + if(g_slist_length(cfg->paths) == 0 && v.all_paths_valid) { /* Still no path set? - use `pwd` */ rm_cfg_prepend_path(cfg, cfg->iwd, is_prefd); } /* Only return success if everything is fine. */ - return all_paths_valid; + return v.all_paths_valid; } static bool rm_cmd_set_outputs(RmSession *session, GError **error) { diff --git a/lib/treemerge.c b/lib/treemerge.c index ab0bd7f9..282a3462 100644 --- a/lib/treemerge.c +++ b/lib/treemerge.c @@ -189,7 +189,7 @@ static bool rm_tm_count_files(RmTrie *count_tree, const RmCfg *const cfg) { const char **const path_vec = malloc(sizeof(*path_vec) * (path_count + 1)); if(!path_vec) { - rm_log_error(_("Failed to allocate memory. Out of memory?")); + rm_log_perror(_("Failed to allocate memory")); return false; } diff --git a/po/de.po b/po/de.po index ac64b56a..feb59eb5 100644 --- a/po/de.po +++ b/po/de.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: rmlint 2.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-10-06 00:43+0000\n" +"POT-Creation-Date: 2018-10-15 04:46+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -1015,8 +1015,8 @@ msgstr "" msgid "Failed to complete setup for merging directories" msgstr "" -#: lib/treemerge.c -msgid "Failed to allocate memory. Out of memory?" +#: lib/treemerge.c lib/cmdline.c +msgid "Failed to allocate memory" msgstr "" #: lib/cmdline.c @@ -1038,6 +1038,14 @@ msgstr "" msgid "Could not get metadata for path \"%s\": %s" msgstr "" +#: lib/cmdline.c +msgid "Failed to read stdin" +msgstr "" + +#: lib/cmdline.c +msgid "Could not process path arguments" +msgstr "" + #~ msgid "%s%15" #~ msgstr "%s%15" diff --git a/po/es.po b/po/es.po index f24d92bb..d0a4c46e 100644 --- a/po/es.po +++ b/po/es.po @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: rmlint 2.4.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-10-06 00:43+0000\n" +"POT-Creation-Date: 2018-10-15 04:46+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -992,8 +992,8 @@ msgstr "" msgid "Failed to complete setup for merging directories" msgstr "" -#: lib/treemerge.c -msgid "Failed to allocate memory. Out of memory?" +#: lib/treemerge.c lib/cmdline.c +msgid "Failed to allocate memory" msgstr "" #: lib/cmdline.c @@ -1015,6 +1015,14 @@ msgstr "" msgid "Could not get metadata for path \"%s\": %s" msgstr "" +#: lib/cmdline.c +msgid "Failed to read stdin" +msgstr "" + +#: lib/cmdline.c +msgid "Could not process path arguments" +msgstr "" + #~ msgid "caching is not supported due to missing json-glib library." #~ msgstr "" #~ "el cacheo no es soportado debido a la librería inexistente json-glib" diff --git a/po/fr.po b/po/fr.po index 5dcbdc80..2b2c58c1 100644 --- a/po/fr.po +++ b/po/fr.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: rmlint 2.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-10-06 00:43+0000\n" +"POT-Creation-Date: 2018-10-15 04:46+0000\n" "PO-Revision-Date: 2014-12-02 13:37+0100\n" "Last-Translator: F. \n" "Language-Team: none\n" @@ -988,8 +988,8 @@ msgstr "" msgid "Failed to complete setup for merging directories" msgstr "" -#: lib/treemerge.c -msgid "Failed to allocate memory. Out of memory?" +#: lib/treemerge.c lib/cmdline.c +msgid "Failed to allocate memory" msgstr "" #: lib/cmdline.c @@ -1011,6 +1011,14 @@ msgstr "" msgid "Could not get metadata for path \"%s\": %s" msgstr "" +#: lib/cmdline.c +msgid "Failed to read stdin" +msgstr "" + +#: lib/cmdline.c +msgid "Could not process path arguments" +msgstr "" + #~ msgid "caching is not supported due to missing json-glib library." #~ msgstr "Cache non supporté, librairie json-glib manquante." diff --git a/po/rmlint.pot b/po/rmlint.pot index 7e3e1ce4..94b92658 100644 --- a/po/rmlint.pot +++ b/po/rmlint.pot @@ -6,9 +6,9 @@ #, fuzzy msgid "" msgstr "" -"Project-Id-Version: rmlint 2.6.2\n" +"Project-Id-Version: rmlint 2.7.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-10-06 00:43+0000\n" +"POT-Creation-Date: 2018-10-15 04:46+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -937,8 +937,8 @@ msgstr "" msgid "Failed to complete setup for merging directories" msgstr "" -#: lib/treemerge.c -msgid "Failed to allocate memory. Out of memory?" +#: lib/treemerge.c lib/cmdline.c +msgid "Failed to allocate memory" msgstr "" #: lib/cmdline.c @@ -959,3 +959,11 @@ msgstr "" #, c-format msgid "Could not get metadata for path \"%s\": %s" msgstr "" + +#: lib/cmdline.c +msgid "Failed to read stdin" +msgstr "" + +#: lib/cmdline.c +msgid "Could not process path arguments" +msgstr "" From 402401c3fb66cc706119e18c113ac24086334dac Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Sat, 29 Sep 2018 18:01:59 +0000 Subject: [PATCH 06/18] lib/cfg.h: Remove member `read_stdin' It's only used locally in one function, so it can just be a local variable in that one function. --- lib/cfg.h | 1 - lib/cmdline.c | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cfg.h b/lib/cfg.h index cb0e4909..94e2e1da 100644 --- a/lib/cfg.h +++ b/lib/cfg.h @@ -73,7 +73,6 @@ typedef struct RmCfg { gboolean progress_enabled; gboolean list_mounts; gboolean replay; - gboolean read_stdin; gboolean read_stdin0; gboolean no_backup; diff --git a/lib/cmdline.c b/lib/cmdline.c index 56bb81a5..bafb48fc 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1194,6 +1194,7 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_assert(cfg); + bool read_stdin = false; bool is_prefd = false; rm_cmd_set_paths_vars v = { .cfg = cfg, @@ -1203,7 +1204,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { /* Check the directory to be valid */ for(int i = 0; paths && paths[i]; ++i) { if(strcmp(paths[i], "-") == 0) { - cfg->read_stdin = true; + read_stdin = true; /* remember whether to treat stdin paths as preferred paths */ v.stdin_paths_preferred = is_prefd; } else if(strcmp(paths[i], "//") == 0) { @@ -1216,7 +1217,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_strfreev(paths); - if(cfg->read_stdin || cfg->read_stdin0) { + if(read_stdin || cfg->read_stdin0) { /* option '-' means read paths from stdin */ if(!rm_cmd_set_paths_from_stdin(&v)) { rm_log_error_line(_("Could not process path arguments")); From 0cc9301907b052a1aaa6e37a2c28a0f431f81bbb Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Sat, 29 Sep 2018 18:09:16 +0000 Subject: [PATCH 07/18] lib/cmdline.c: Consolidate the logic around whether to read paths from `stdin' --- lib/cmdline.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index bafb48fc..32b4604b 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1148,6 +1148,7 @@ static bool rm_cmd_set_cmdline(RmCfg *cfg, int argc, char **argv) { typedef struct rm_cmd_set_paths_vars { RmCfg *const cfg; bool stdin_paths_preferred; + const bool null_separated; bool all_paths_valid; } rm_cmd_set_paths_vars; @@ -1158,9 +1159,8 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { RmCfg *const cfg = v->cfg; const bool is_prefd = v->stdin_paths_preferred; - const bool null_separated = cfg->read_stdin0; - char delim = null_separated ? 0 : '\n'; + char delim = v->null_separated ? 0 : '\n'; size_t buf_len; char *path_buf = malloc(buf_len = PATH_MAX); @@ -1194,10 +1194,12 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_assert(cfg); - bool read_stdin = false; + const bool read_stdin0 = cfg->read_stdin0; + bool read_stdin = read_stdin0; bool is_prefd = false; rm_cmd_set_paths_vars v = { .cfg = cfg, + .null_separated = read_stdin0, .all_paths_valid = true }; @@ -1217,7 +1219,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_strfreev(paths); - if(read_stdin || cfg->read_stdin0) { + if(read_stdin) { /* option '-' means read paths from stdin */ if(!rm_cmd_set_paths_from_stdin(&v)) { rm_log_error_line(_("Could not process path arguments")); From fc05f46bcf5c73865ae60d88b3bb7f40baa86fa7 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Wed, 3 Oct 2018 16:00:28 +0000 Subject: [PATCH 08/18] lib/cmdline.c: Move constant loop condition (`paths') out of the loop The compiler probably already does this, but I find it clearer and more satisfying to do this explicitly, especially since it allows avoiding a call to `g_strfreev()', rather than relying on that function to check `paths' for us again. For the sake of clarity in the diff, the existing code has not yet been indented to reflect the fact that it's part of the `if' statement's consequent. --- lib/cmdline.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 32b4604b..f29a2c35 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1203,8 +1203,8 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { .all_paths_valid = true }; - /* Check the directory to be valid */ - for(int i = 0; paths && paths[i]; ++i) { + if(paths) { + for(int i = 0; paths[i]; ++i) { if(strcmp(paths[i], "-") == 0) { read_stdin = true; /* remember whether to treat stdin paths as preferred paths */ @@ -1218,6 +1218,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { } g_strfreev(paths); + } if(read_stdin) { /* option '-' means read paths from stdin */ From af0381e6efad1d13192a8e83a94e5a6d0c8b6912 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Wed, 3 Oct 2018 16:03:32 +0000 Subject: [PATCH 09/18] lib/cmdline.c: Indent code to reflect the fact that it's in a block You can check that only white space changed: $ git diff HEAD^ --ignore-space-change && echo Nothing\ changed Nothing changed --- lib/cmdline.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index f29a2c35..60e33c68 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1204,20 +1204,20 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { }; if(paths) { - for(int i = 0; paths[i]; ++i) { - if(strcmp(paths[i], "-") == 0) { - read_stdin = true; - /* remember whether to treat stdin paths as preferred paths */ - v.stdin_paths_preferred = is_prefd; - } else if(strcmp(paths[i], "//") == 0) { - /* the '//' separator separates non-preferred paths from preferred */ - is_prefd = !is_prefd; - } else { - v.all_paths_valid &= rm_cfg_prepend_path(cfg, paths[i], is_prefd); + for(int i = 0; paths[i]; ++i) { + if(strcmp(paths[i], "-") == 0) { + read_stdin = true; + /* remember whether to treat stdin paths as preferred paths */ + v.stdin_paths_preferred = is_prefd; + } else if(strcmp(paths[i], "//") == 0) { + /* the '//' separator separates non-preferred paths from preferred */ + is_prefd = !is_prefd; + } else { + v.all_paths_valid &= rm_cfg_prepend_path(cfg, paths[i], is_prefd); + } } - } - g_strfreev(paths); + g_strfreev(paths); } if(read_stdin) { From dba286ccf4ce5c04de4ff5af6337dc3d6f279d75 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Wed, 3 Oct 2018 17:31:35 +0000 Subject: [PATCH 10/18] lib/cmdline.c: Use a pointer rather than an index integer for looping --- lib/cmdline.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 60e33c68..1bacd267 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1204,16 +1204,16 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { }; if(paths) { - for(int i = 0; paths[i]; ++i) { - if(strcmp(paths[i], "-") == 0) { + for(char *path, **list = paths; (path = *list); ++list) { + if(strcmp(path, "-") == 0) { read_stdin = true; /* remember whether to treat stdin paths as preferred paths */ v.stdin_paths_preferred = is_prefd; - } else if(strcmp(paths[i], "//") == 0) { + } else if(strcmp(path, "//") == 0) { /* the '//' separator separates non-preferred paths from preferred */ is_prefd = !is_prefd; } else { - v.all_paths_valid &= rm_cfg_prepend_path(cfg, paths[i], is_prefd); + v.all_paths_valid &= rm_cfg_prepend_path(cfg, path, is_prefd); } } From 38b065fcb339a9a4948b1587689a4f1e131cf534 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Wed, 3 Oct 2018 17:37:26 +0000 Subject: [PATCH 11/18] lib/cmdline.c: Free each command-line path as it is processed. On each iteration of the loop, a path string is freed when processing is finished, and then the whole list of strings is simply freed without having to iterate through the list again to free each string. --- lib/cmdline.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 1bacd267..e07d0e39 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1215,9 +1215,9 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { } else { v.all_paths_valid &= rm_cfg_prepend_path(cfg, path, is_prefd); } + g_free(path); } - - g_strfreev(paths); + g_free(paths); } if(read_stdin) { From 2ba401f52df850e57c333f13568824d35055ef69 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Thu, 4 Oct 2018 06:25:11 +0000 Subject: [PATCH 12/18] lib/cfg-funcs.h: Move `cfg->replay' and `cfg->path_count++' These variables were handled directly in `rm_cfg_prepend_path()'; now, they're handled directly in `rm_cmd_set_paths*()'. This has allowed for optimizing the call to `rm_cfg_prepend_path()' that is used for adding the current working directory when no other path has been supplied by the user; there's no reason to check whether the current working directory is a JSON file. --- lib/cfg-funcs.h | 6 ++++-- lib/cmdline.c | 12 +++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/cfg-funcs.h b/lib/cfg-funcs.h index 283df3c7..b17672f0 100644 --- a/lib/cfg-funcs.h +++ b/lib/cfg-funcs.h @@ -129,16 +129,18 @@ static INLINE bool rm_cfg_prepend_path( RmCfg *const cfg, const char *const path, + const unsigned int index, + const bool replay, const bool preferred ) { g_assert(cfg); char *real_path; if(rm_path_is_valid(path, &real_path)) { rm_path_prepend( - (cfg->replay && rm_path_is_json(path)) ? + (replay && rm_path_is_json(path)) ? &cfg->json_paths : &cfg->paths, real_path, - cfg->path_count++, + index, preferred ); return true; } return false; diff --git a/lib/cmdline.c b/lib/cmdline.c index e07d0e39..e1130d70 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1178,7 +1178,9 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { if(path_buf[path_len - 1] == delim) { path_buf[path_len - 1] = 0; } - v->all_paths_valid &= rm_cfg_prepend_path(cfg, path_buf, is_prefd); + v->all_paths_valid &= rm_cfg_prepend_path( + cfg, path_buf, cfg->path_count++, cfg->replay, is_prefd + ); } } @@ -1213,7 +1215,9 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { /* the '//' separator separates non-preferred paths from preferred */ is_prefd = !is_prefd; } else { - v.all_paths_valid &= rm_cfg_prepend_path(cfg, path, is_prefd); + v.all_paths_valid &= rm_cfg_prepend_path( + cfg, path, cfg->path_count++, cfg->replay, is_prefd + ); } g_free(path); } @@ -1230,7 +1234,9 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { if(g_slist_length(cfg->paths) == 0 && v.all_paths_valid) { /* Still no path set? - use `pwd` */ - rm_cfg_prepend_path(cfg, cfg->iwd, is_prefd); + rm_cfg_prepend_path( + cfg, cfg->iwd, cfg->path_count++, /* replay */ false, is_prefd + ); } /* Only return success if everything is fine. */ From c2557c94d2546ec2ba72696be5b330e974bbdfd5 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Thu, 4 Oct 2018 06:31:43 +0000 Subject: [PATCH 13/18] lib/cmdline.c: Move `cfg->replay' and `cfg->path_count++' out of loops Though an optimizing compiler might do this already, one might as well just do it explicitly, so as not to depend on the whims of any particular compiler. --- lib/cmdline.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index e1130d70..00013ada 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1147,13 +1147,17 @@ static bool rm_cmd_set_cmdline(RmCfg *cfg, int argc, char **argv) { typedef struct rm_cmd_set_paths_vars { RmCfg *const cfg; + unsigned int index; bool stdin_paths_preferred; const bool null_separated; bool all_paths_valid; } rm_cmd_set_paths_vars; static INLINE -bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { +bool rm_cmd_set_paths_from_stdin( + rm_cmd_set_paths_vars *const v, + const bool replay +) { g_assert(v); g_assert(v->cfg); @@ -1179,7 +1183,7 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { path_buf[path_len - 1] = 0; } v->all_paths_valid &= rm_cfg_prepend_path( - cfg, path_buf, cfg->path_count++, cfg->replay, is_prefd + cfg, path_buf, v->index++, replay, is_prefd ); } } @@ -1196,11 +1200,13 @@ bool rm_cmd_set_paths_from_stdin(rm_cmd_set_paths_vars *const v) { static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_assert(cfg); + const bool replay = cfg->replay; const bool read_stdin0 = cfg->read_stdin0; bool read_stdin = read_stdin0; bool is_prefd = false; rm_cmd_set_paths_vars v = { .cfg = cfg, + .index = cfg->path_count, .null_separated = read_stdin0, .all_paths_valid = true }; @@ -1216,7 +1222,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { is_prefd = !is_prefd; } else { v.all_paths_valid &= rm_cfg_prepend_path( - cfg, path, cfg->path_count++, cfg->replay, is_prefd + cfg, path, v.index++, replay, is_prefd ); } g_free(path); @@ -1226,7 +1232,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { if(read_stdin) { /* option '-' means read paths from stdin */ - if(!rm_cmd_set_paths_from_stdin(&v)) { + if(!rm_cmd_set_paths_from_stdin(&v, replay)) { rm_log_error_line(_("Could not process path arguments")); return false; } @@ -1235,10 +1241,12 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { if(g_slist_length(cfg->paths) == 0 && v.all_paths_valid) { /* Still no path set? - use `pwd` */ rm_cfg_prepend_path( - cfg, cfg->iwd, cfg->path_count++, /* replay */ false, is_prefd + cfg, cfg->iwd, v.index++, /* replay */ false, is_prefd ); } + cfg->path_count = v.index; + /* Only return success if everything is fine. */ return v.all_paths_valid; } From 43c05a62ee323cc7e3561ec22dc0e11706b979c0 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Mon, 15 Oct 2018 02:09:32 +0000 Subject: [PATCH 14/18] lib/cfg.h: Use `RmCfg::path_count' more purposefully Now, member `path_count' of a struct `RmCfg' has a more guaranteed and expected purpose: During and after the user-input processing of non-option path arguments, it represents the number of items in the associated list `paths'. This has allowed for the removal of a call to `g_slist_length()'. --- lib/cfg-funcs.h | 23 ++++++++++++++++------- lib/cfg.h | 15 ++++++--------- lib/cmdline.c | 8 ++++---- 3 files changed, 26 insertions(+), 20 deletions(-) diff --git a/lib/cfg-funcs.h b/lib/cfg-funcs.h index b17672f0..9b218c31 100644 --- a/lib/cfg-funcs.h +++ b/lib/cfg-funcs.h @@ -125,6 +125,14 @@ bool rm_cfg_prepend_json( } return false; } +#define PREPEND_TO(list) \ + rm_path_prepend( \ + &(list), \ + real_path, \ + index, \ + preferred \ + ) + static INLINE bool rm_cfg_prepend_path( RmCfg *const cfg, @@ -136,16 +144,17 @@ bool rm_cfg_prepend_path( g_assert(cfg); char *real_path; if(rm_path_is_valid(path, &real_path)) { - rm_path_prepend( - (replay && rm_path_is_json(path)) ? - &cfg->json_paths : &cfg->paths, - real_path, - index, - preferred - ); return true; + if(replay && rm_path_is_json(path)) { + PREPEND_TO(cfg->json_paths); + } else { + PREPEND_TO(cfg->paths); + ++(cfg->path_count); + } return true; } return false; } +#undef PREPEND_TO + static INLINE void rm_cfg_free_paths(RmCfg *const cfg) { g_assert(cfg); diff --git a/lib/cfg.h b/lib/cfg.h index 94e2e1da..63e2cd3e 100644 --- a/lib/cfg.h +++ b/lib/cfg.h @@ -100,16 +100,13 @@ typedef struct RmCfg { * * + To record a unique index for each path supplied by the * user; a path's index represents the number of paths that - * were already processed. This is always the case. + * were already processed. This is the case DURING the + * processing of user-input options (such as '--replay'). * - * + To provide quick access to the length of its associated - * RmCfg::paths list. This is only the case when NOT running - * in "--replay" mode; when running in "--replay" mode, it - * just represents the total number of paths that have been - * supplied by the user, i.e., the sums of the lengths of - * the associated lists RmCfg::{paths,json_paths}, which is - * not meant to be a useful number to know, and is simply a - * byproduct of calculating path indicies. + * + To provide quick access to the length of its associated + * RmCfg::paths list. This is the case AFTER the processing + * of user-input options (including during the processing of + * non-option paths from the command line or stdin). */ guint path_count; diff --git a/lib/cmdline.c b/lib/cmdline.c index 00013ada..4aa0ba3b 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1211,6 +1211,8 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { .all_paths_valid = true }; + cfg->path_count = 0; + if(paths) { for(char *path, **list = paths; (path = *list); ++list) { if(strcmp(path, "-") == 0) { @@ -1238,15 +1240,13 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { } } - if(g_slist_length(cfg->paths) == 0 && v.all_paths_valid) { + if(cfg->path_count == 0 && v.all_paths_valid) { /* Still no path set? - use `pwd` */ rm_cfg_prepend_path( - cfg, cfg->iwd, v.index++, /* replay */ false, is_prefd + cfg, cfg->iwd, v.index, /* replay */ false, is_prefd ); } - cfg->path_count = v.index; - /* Only return success if everything is fine. */ return v.all_paths_valid; } From 5edd876ba0d197cdedf50113597d5ee672ff7dec Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Thu, 4 Oct 2018 21:18:20 +0000 Subject: [PATCH 15/18] lib/cmdline.c: Abstract out `rm_cmd_set_paths_from_cmdline()' Code from `rm_cmd_set_paths()' has been moved to its own function: rm_cmd_set_paths_from_cmdline() --- lib/cmdline.c | 54 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 4aa0ba3b..baee70c7 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1147,12 +1147,41 @@ static bool rm_cmd_set_cmdline(RmCfg *cfg, int argc, char **argv) { typedef struct rm_cmd_set_paths_vars { RmCfg *const cfg; + char **const paths; unsigned int index; + bool is_prefd; + bool read_stdin; bool stdin_paths_preferred; const bool null_separated; bool all_paths_valid; } rm_cmd_set_paths_vars; +static INLINE +void rm_cmd_set_paths_from_cmdline( + rm_cmd_set_paths_vars *const v, + const bool replay +) { + g_assert(v); + g_assert(v->paths); + + for(char *path, **list = v->paths; (path = *list); ++list) { + if(strcmp(path, "-") == 0) { + v->read_stdin = true; + /* remember whether to treat stdin paths as preferred paths */ + v->stdin_paths_preferred = v->is_prefd; + } else if(strcmp(path, "//") == 0) { + /* the '//' separator separates non-preferred paths from preferred */ + v->is_prefd = !v->is_prefd; + } else { + v->all_paths_valid &= rm_cfg_prepend_path( + v->cfg, path, v->index++, replay, v->is_prefd + ); + } + g_free(path); + } + g_free(v->paths); +} + static INLINE bool rm_cmd_set_paths_from_stdin( rm_cmd_set_paths_vars *const v, @@ -1202,11 +1231,11 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { const bool replay = cfg->replay; const bool read_stdin0 = cfg->read_stdin0; - bool read_stdin = read_stdin0; - bool is_prefd = false; rm_cmd_set_paths_vars v = { .cfg = cfg, + .paths = paths, .index = cfg->path_count, + .read_stdin = read_stdin0, .null_separated = read_stdin0, .all_paths_valid = true }; @@ -1214,25 +1243,10 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { cfg->path_count = 0; if(paths) { - for(char *path, **list = paths; (path = *list); ++list) { - if(strcmp(path, "-") == 0) { - read_stdin = true; - /* remember whether to treat stdin paths as preferred paths */ - v.stdin_paths_preferred = is_prefd; - } else if(strcmp(path, "//") == 0) { - /* the '//' separator separates non-preferred paths from preferred */ - is_prefd = !is_prefd; - } else { - v.all_paths_valid &= rm_cfg_prepend_path( - cfg, path, v.index++, replay, is_prefd - ); - } - g_free(path); - } - g_free(paths); + rm_cmd_set_paths_from_cmdline(&v, replay); } - if(read_stdin) { + if(v.read_stdin) { /* option '-' means read paths from stdin */ if(!rm_cmd_set_paths_from_stdin(&v, replay)) { rm_log_error_line(_("Could not process path arguments")); @@ -1243,7 +1257,7 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { if(cfg->path_count == 0 && v.all_paths_valid) { /* Still no path set? - use `pwd` */ rm_cfg_prepend_path( - cfg, cfg->iwd, v.index, /* replay */ false, is_prefd + cfg, cfg->iwd, v.index, /* replay */ false, v.is_prefd ); } From b8919ced5e35f554c7d2179bcf5fba5b1dfe90fc Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Sun, 7 Oct 2018 17:12:09 +0000 Subject: [PATCH 16/18] lib/cmdline.c: Replace `strcmp()' with direct character comparisons --- lib/cmdline.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index baee70c7..f30b6738 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1165,11 +1165,11 @@ void rm_cmd_set_paths_from_cmdline( g_assert(v->paths); for(char *path, **list = v->paths; (path = *list); ++list) { - if(strcmp(path, "-") == 0) { + if(path[0] == '-' && path[1] == 0) { v->read_stdin = true; /* remember whether to treat stdin paths as preferred paths */ v->stdin_paths_preferred = v->is_prefd; - } else if(strcmp(path, "//") == 0) { + } else if(path[0] == '/' && path[1] == '/' && path[2] == 0) { /* the '//' separator separates non-preferred paths from preferred */ v->is_prefd = !v->is_prefd; } else { From 8f825b075ca9ac8e7f4dc544ee0be529ac3c2995 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Thu, 4 Oct 2018 21:38:23 +0000 Subject: [PATCH 17/18] lib/cmdline.c: Tell compiler to optimize for the value of `replay' When `cfg->replay' is false, there's no reason to check whether a path is a `json' file. --- lib/cmdline.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index f30b6738..7b3fbf73 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1226,6 +1226,17 @@ bool rm_cmd_set_paths_from_stdin( return true; } +#define PROCESS_PATHS(replay) \ + if(paths) { \ + rm_cmd_set_paths_from_cmdline(&v, (replay)); \ + } \ + if(v.read_stdin) { \ + if(!rm_cmd_set_paths_from_stdin(&v, (replay))) { \ + rm_log_error_line(_("Could not process path arguments")); \ + return false; \ + } \ + } + static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_assert(cfg); @@ -1242,16 +1253,10 @@ static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { cfg->path_count = 0; - if(paths) { - rm_cmd_set_paths_from_cmdline(&v, replay); - } - - if(v.read_stdin) { - /* option '-' means read paths from stdin */ - if(!rm_cmd_set_paths_from_stdin(&v, replay)) { - rm_log_error_line(_("Could not process path arguments")); - return false; - } + if(replay) { + PROCESS_PATHS(true); + } else { + PROCESS_PATHS(false); } if(cfg->path_count == 0 && v.all_paths_valid) { From c5b7b3e83c95ee1ec7dadd8a1075f8852f522128 Mon Sep 17 00:00:00 2001 From: Michael Witten Date: Tue, 16 Oct 2018 18:42:40 +0000 Subject: [PATCH 18/18] lib/cmdline.c: INLINE `rm_cmd_set_paths()' It's a static function that is used in only place. --- lib/cmdline.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cmdline.c b/lib/cmdline.c index 7b3fbf73..56d07a8e 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -1237,7 +1237,8 @@ bool rm_cmd_set_paths_from_stdin( } \ } -static bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { +static INLINE +bool rm_cmd_set_paths(RmCfg *const cfg, char **const paths) { g_assert(cfg); const bool replay = cfg->replay;