[pacman-dev] [PATCH 1/7] pacman/util: remove strsplit

newer
[pacman-dev] [PATCH] libalpm: move...

Andrew Gregory

12 Oct 2013 12 Oct '13

5:32 p.m.

strsplit was used in only one place and did the same thing as strtok. Signed-off-by: Andrew Gregory --- src/pacman/pacman.c | 10 +++++----- src/pacman/util.c | 34 ---------------------------------- src/pacman/util.h | 1 - 3 files changed, 5 insertions(+), 40 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 24fd57f..d2114e6 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -325,14 +325,14 @@ static void handler(int signum) static int parsearg_util_addlist(alpm_list_t **list) { - alpm_list_t *split, *item; + char *i, *save; check_optarg(); - split = strsplit(optarg, ','); - for(item = split; item; item = alpm_list_next(item)) { - *list = alpm_list_add(*list, item->data); + + for(i = strtok_r(optarg, ",", &save); i; i = strtok_r(NULL, ",", &save)) { + *list = alpm_list_add(*list, strdup(i)); } - alpm_list_free(split); + return 0; } diff --git a/src/pacman/util.c b/src/pacman/util.c index 6da1cd4..0e9a420 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -378,40 +378,6 @@ char *strreplace(const char *str, const char *needle, const char *replace) return newstr; } -/** Splits a string into a list of strings using the chosen character as - * a delimiter. - * - * @param str the string to split - * @param splitchar the character to split at - * - * @return a list containing the duplicated strings - */ -alpm_list_t *strsplit(const char *str, const char splitchar) -{ - alpm_list_t *list = NULL; - const char *prev = str; - char *dup = NULL; - - while((str = strchr(str, splitchar))) { - dup = strndup(prev, (size_t)(str - prev)); - if(dup == NULL) { - return NULL; - } - list = alpm_list_add(list, dup); - - str++; - prev = str; - } - - dup = strdup(prev); - if(dup == NULL) { - return NULL; - } - list = alpm_list_add(list, dup); - - return list; -} - static size_t string_length(const char *s) { int len; diff --git a/src/pacman/util.h b/src/pacman/util.h index 0a2a6f7..e2297f8 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -54,7 +54,6 @@ int rmrf(const char *path); void indentprint(const char *str, unsigned short indent, unsigned short cols); size_t strtrim(char *str); char *strreplace(const char *str, const char *needle, const char *replace); -alpm_list_t *strsplit(const char *str, const char splitchar); void string_display(const char *title, const char *string, unsigned short cols); double humanize_size(off_t bytes, const char target_unit, int precision, const char **label); -- 1.8.4

Show replies by thread

Andrew Gregory

12 Oct 12 Oct

5:32 p.m.

New subject: [pacman-dev] [PATCH 2/7] use non-ascii identifiers for optflags

Removes the overlap between optflags for different operations that allowed non-sensical combinations of flags such as: $ pacman -Si --changelog $package --changelog is -c, meaning --clean for -S $ pacman -Q --sysupgrade --sysupgrade is -u, meaning --upgrades for -Q Also add a few missing braces. Original-work-by: Dave Reisner Signed-off-by: Andrew Gregory --- src/pacman/conf.h | 31 ++++++- src/pacman/pacman.c | 245 +++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 206 insertions(+), 70 deletions(-) diff --git a/src/pacman/conf.h b/src/pacman/conf.h index dcd3204..e263d7c 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -146,7 +146,36 @@ enum { OP_GPGDIR, OP_DBONLY, OP_FORCE, - OP_COLOR + OP_COLOR, + OP_DBPATH, + OP_CASCADE, + OP_CHANGELOG, + OP_CLEAN, + OP_NODEPS, + OP_DEPS, + OP_EXPLICIT, + OP_GROUPS, + OP_HELP, + OP_INFO, + OP_CHECK, + OP_LIST, + OP_FOREIGN, + OP_NATIVE, + OP_NOSAVE, + OP_OWNS, + OP_FILE, + OP_PRINT, + OP_QUIET, + OP_ROOT, + OP_RECURSIVE, + OP_SEARCH, + OP_UNREQUIRED, + OP_UPGRADES, + OP_SYSUPGRADE, + OP_UNNEEDED, + OP_VERBOSE, + OP_DOWNLOADONLY, + OP_REFRESH }; /* clean method */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index d2114e6..93baa44 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -446,14 +446,25 @@ static int parsearg_global(int opt) check_optarg(); config->logfile = strndup(optarg, PATH_MAX); break; - case OP_NOCONFIRM: config->noconfirm = 1; break; + case OP_NOCONFIRM: + config->noconfirm = 1; + break; + case OP_DBPATH: case 'b': check_optarg(); config->dbpath = strdup(optarg); break; - case 'r': check_optarg(); config->rootdir = strdup(optarg); break; - case 'v': (config->verbose)++; break; - default: return 1; + case OP_ROOT: + case 'r': + check_optarg(); + config->rootdir = strdup(optarg); + break; + case OP_VERBOSE: + case 'v': + (config->verbose)++; + break; + default: + return 1; } return 0; } @@ -471,22 +482,67 @@ static int parsearg_database(int opt) static int parsearg_query(int opt) { switch(opt) { - case 'c': config->op_q_changelog = 1; break; - case 'd': config->op_q_deps = 1; break; - case 'e': config->op_q_explicit = 1; break; - case 'g': (config->group)++; break; - case 'i': (config->op_q_info)++; break; - case 'k': (config->op_q_check)++; break; - case 'l': config->op_q_list = 1; break; - case 'm': config->op_q_locality |= PKG_LOCALITY_LOCAL; break; - case 'n': config->op_q_locality |= PKG_LOCALITY_FOREIGN; break; - case 'o': config->op_q_owns = 1; break; - case 'p': config->op_q_isfile = 1; break; - case 'q': config->quiet = 1; break; - case 's': config->op_q_search = 1; break; - case 't': (config->op_q_unrequired)++; break; - case 'u': config->op_q_upgrade = 1; break; - default: return 1; + case OP_CHANGELOG: + case 'c': + config->op_q_changelog = 1; + break; + case OP_DEPS: + case 'd': + config->op_q_deps = 1; + break; + case OP_EXPLICIT: + case 'e': + config->op_q_explicit = 1; + break; + case OP_GROUPS: + case 'g': + (config->group)++; + break; + case OP_INFO: + case 'i': + (config->op_q_info)++; + break; + case OP_CHECK: + case 'k': + (config->op_q_check)++; + break; + case OP_LIST: + case 'l': + config->op_q_list = 1; + break; + case OP_FOREIGN: + case 'm': + config->op_q_locality |= PKG_LOCALITY_LOCAL; + break; + case OP_NATIVE: + case 'n': + config->op_q_locality |= PKG_LOCALITY_FOREIGN; + break; + case OP_OWNS: + case 'o': + config->op_q_owns = 1; + break; + case OP_FILE: + case 'p': + config->op_q_isfile = 1; + break; + case OP_QUIET: + case 'q': + config->quiet = 1; + break; + case OP_SEARCH: + case 's': + config->op_q_search = 1; + break; + case OP_UNREQUIRED: + case 't': + (config->op_q_unrequired)++; + break; + case OP_UPGRADES: + case 'u': + config->op_q_upgrade = 1; break; + default: + return 1; } return 0; } @@ -495,6 +551,7 @@ static int parsearg_query(int opt) static int parsearg_trans(int opt) { switch(opt) { + case OP_NODEPS: case 'd': if(config->flags & ALPM_TRANS_FLAG_NODEPVERSION) { config->flags |= ALPM_TRANS_FLAG_NODEPS; @@ -502,26 +559,44 @@ static int parsearg_trans(int opt) config->flags |= ALPM_TRANS_FLAG_NODEPVERSION; } break; - case OP_DBONLY: config->flags |= ALPM_TRANS_FLAG_DBONLY; break; - case OP_NOPROGRESSBAR: config->noprogressbar = 1; break; - case OP_NOSCRIPTLET: config->flags |= ALPM_TRANS_FLAG_NOSCRIPTLET; break; - case 'p': config->print = 1; break; + case OP_DBONLY: + config->flags |= ALPM_TRANS_FLAG_DBONLY; + break; + case OP_NOPROGRESSBAR: + config->noprogressbar = 1; + break; + case OP_NOSCRIPTLET: + config->flags |= ALPM_TRANS_FLAG_NOSCRIPTLET; + break; + case OP_PRINT: + case 'p': + config->print = 1; + break; case OP_PRINTFORMAT: check_optarg(); config->print_format = strdup(optarg); break; - default: return 1; + default: + return 1; } return 0; } static int parsearg_remove(int opt) { - if(parsearg_trans(opt) == 0) + if(parsearg_trans(opt) == 0) { return 0; + } switch(opt) { - case 'c': config->flags |= ALPM_TRANS_FLAG_CASCADE; break; - case 'n': config->flags |= ALPM_TRANS_FLAG_NOSAVE; break; + case OP_CASCADE: + case 'c': + config->flags |= ALPM_TRANS_FLAG_CASCADE; + break; + case OP_NOSAVE: + case 'n': + config->flags |= ALPM_TRANS_FLAG_NOSAVE; + break; + case OP_RECURSIVE: case 's': if(config->flags & ALPM_TRANS_FLAG_RECURSE) { config->flags |= ALPM_TRANS_FLAG_RECURSEALL; @@ -529,8 +604,12 @@ static int parsearg_remove(int opt) config->flags |= ALPM_TRANS_FLAG_RECURSE; } break; - case 'u': config->flags |= ALPM_TRANS_FLAG_UNNEEDED; break; - default: return 1; + case OP_UNNEEDED: + case 'u': + config->flags |= ALPM_TRANS_FLAG_UNNEEDED; + break; + default: + return 1; } return 0; } @@ -538,8 +617,9 @@ static int parsearg_remove(int opt) /* options common to -S -U */ static int parsearg_upgrade(int opt) { - if(parsearg_trans(opt) == 0) + if(parsearg_trans(opt) == 0) { return 0; + } switch(opt) { case OP_FORCE: config->flags |= ALPM_TRANS_FLAG_FORCE; break; case OP_ASDEPS: config->flags |= ALPM_TRANS_FLAG_ALLDEPS; break; @@ -558,23 +638,50 @@ static int parsearg_upgrade(int opt) static int parsearg_sync(int opt) { - if(parsearg_upgrade(opt) == 0) + if(parsearg_upgrade(opt) == 0) { return 0; + } switch(opt) { - case 'c': (config->op_s_clean)++; break; - case 'g': (config->group)++; break; - case 'i': (config->op_s_info)++; break; - case 'l': config->op_q_list = 1; break; - case 'q': config->quiet = 1; break; - case 's': config->op_s_search = 1; break; - case 'u': (config->op_s_upgrade)++; break; + case OP_CLEAN: + case 'c': + (config->op_s_clean)++; + break; + case OP_GROUPS: + case 'g': + (config->group)++; + break; + case OP_INFO: + case 'i': + (config->op_s_info)++; + break; + case OP_LIST: + case 'l': + config->op_q_list = 1; + break; + case OP_QUIET: + case 'q': + config->quiet = 1; + break; + case OP_SEARCH: + case 's': + config->op_s_search = 1; + break; + case OP_SYSUPGRADE: + case 'u': + (config->op_s_upgrade)++; + break; + case OP_DOWNLOADONLY: case 'w': config->op_s_downloadonly = 1; config->flags |= ALPM_TRANS_FLAG_DOWNLOADONLY; config->flags |= ALPM_TRANS_FLAG_NOCONFLICTS; break; - case 'y': (config->op_s_sync)++; break; - default: return 1; + case OP_REFRESH: + case 'y': + (config->op_s_sync)++; + break; + default: + return 1; } return 0; } @@ -599,36 +706,36 @@ static int parseargs(int argc, char *argv[]) {"deptest", no_argument, 0, 'T'}, /* used by makepkg */ {"upgrade", no_argument, 0, 'U'}, {"version", no_argument, 0, 'V'}, - {"dbpath", required_argument, 0, 'b'}, - {"cascade", no_argument, 0, 'c'}, - {"changelog", no_argument, 0, 'c'}, - {"clean", no_argument, 0, 'c'}, - {"nodeps", no_argument, 0, 'd'}, - {"deps", no_argument, 0, 'd'}, - {"explicit", no_argument, 0, 'e'}, - {"groups", no_argument, 0, 'g'}, {"help", no_argument, 0, 'h'}, - {"info", no_argument, 0, 'i'}, - {"check", no_argument, 0, 'k'}, - {"list", no_argument, 0, 'l'}, - {"foreign", no_argument, 0, 'm'}, - {"native", no_argument, 0, 'n'}, - {"nosave", no_argument, 0, 'n'}, - {"owns", no_argument, 0, 'o'}, - {"file", no_argument, 0, 'p'}, - {"print", no_argument, 0, 'p'}, - {"quiet", no_argument, 0, 'q'}, - {"root", required_argument, 0, 'r'}, - {"recursive", no_argument, 0, 's'}, - {"search", no_argument, 0, 's'}, - {"unrequired", no_argument, 0, 't'}, - {"upgrades", no_argument, 0, 'u'}, - {"sysupgrade", no_argument, 0, 'u'}, - {"unneeded", no_argument, 0, 'u'}, - {"verbose", no_argument, 0, 'v'}, - {"downloadonly", no_argument, 0, 'w'}, - {"refresh", no_argument, 0, 'y'}, + {"dbpath", required_argument, 0, OP_DBPATH}, + {"cascade", no_argument, 0, OP_CASCADE}, + {"changelog", no_argument, 0, OP_CHANGELOG}, + {"clean", no_argument, 0, OP_CLEAN}, + {"nodeps", no_argument, 0, OP_NODEPS}, + {"deps", no_argument, 0, OP_DEPS}, + {"explicit", no_argument, 0, OP_EXPLICIT}, + {"groups", no_argument, 0, OP_GROUPS}, + {"info", no_argument, 0, OP_INFO}, + {"check", no_argument, 0, OP_CHECK}, + {"list", no_argument, 0, OP_LIST}, + {"foreign", no_argument, 0, OP_FOREIGN}, + {"native", no_argument, 0, OP_NATIVE}, + {"nosave", no_argument, 0, OP_NOSAVE}, + {"owns", no_argument, 0, OP_OWNS}, + {"file", no_argument, 0, OP_FILE}, + {"print", no_argument, 0, OP_PRINT}, + {"quiet", no_argument, 0, OP_QUIET}, + {"root", required_argument, 0, OP_ROOT}, + {"recursive", no_argument, 0, OP_RECURSIVE}, + {"search", no_argument, 0, OP_SEARCH}, + {"unrequired", no_argument, 0, OP_UNREQUIRED}, + {"upgrades", no_argument, 0, OP_UPGRADES}, + {"sysupgrade", no_argument, 0, OP_SYSUPGRADE}, + {"unneeded", no_argument, 0, OP_UNNEEDED}, + {"verbose", no_argument, 0, OP_VERBOSE}, + {"downloadonly", no_argument, 0, OP_DOWNLOADONLY}, + {"refresh", no_argument, 0, OP_REFRESH}, {"noconfirm", no_argument, 0, OP_NOCONFIRM}, {"config", required_argument, 0, OP_CONFIG}, {"ignore", required_argument, 0, OP_IGNORE}, -- 1.8.4

Andrew Gregory

5:32 p.m.

New subject: [pacman-dev] [PATCH 3/7] include invalid options in error messages

On invalid combinations of flags we were only printing the unhelpfully vague message "invalid option". Signed-off-by: Andrew Gregory --- src/pacman/conf.h | 3 ++- src/pacman/pacman.c | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/pacman/conf.h b/src/pacman/conf.h index e263d7c..45e48c0 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -128,7 +128,8 @@ enum { /* Long Operations */ enum { - OP_NOCONFIRM = 1000, + OP_LONG_FLAG_MIN = 1000, + OP_NOCONFIRM, OP_CONFIG, OP_IGNORE, OP_DEBUG, diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 93baa44..308ff38 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -828,7 +828,12 @@ static int parseargs(int argc, char *argv[]) result = parsearg_global(opt); if(result != 0) { /* global option parsing failed, abort */ - pm_printf(ALPM_LOG_ERROR, _("invalid option\n")); + if(opt < OP_LONG_FLAG_MIN) { + pm_printf(ALPM_LOG_ERROR, _("invalid option '-%c'\n"), opt); + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid option '--%s'\n"), + opts[option_index].name); + } return result; } } -- 1.8.4

Andrew Gregory

5:32 p.m.

New subject: [pacman-dev] [PATCH 4/7] imply --print from --print-format

--print-format is totally useless without --print. Implying --print will also save us the hassle of checking it when we add transaction option validation. Signed-off-by: Andrew Gregory --- doc/pacman.8.txt | 2 +- src/pacman/pacman.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 574995c..ccc0a01 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -204,7 +204,7 @@ Transaction Options (apply to '-S', '-R' and '-U') *\--print-format* <format>:: Specify a printf-like format to control the output of the '\--print' operation. The possible attributes are: %n for pkgname, %v for pkgver, - %l for location, %r for repo, and %s for size. + %l for location, %r for repo, and %s for size. Implies '\--print'. Upgrade Options (apply to '-S' and '-U')[[UO]] -------------------------------------------- diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 308ff38..4e91608 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -574,6 +574,7 @@ static int parsearg_trans(int opt) break; case OP_PRINTFORMAT: check_optarg(); + config->print = 1; config->print_format = strdup(optarg); break; default: -- 1.8.4

Andrew Gregory

5:32 p.m.

New subject: [pacman-dev] [PATCH 5/7] rename PKG_LOCALITY_LOCAL -> PKG_LOCALITY_NATIVE

PKG_LOCALITY_LOCAL was confusing because the enum is used with -Q, so all packages are "local". Also reversed the config->op_q_locality assignment so that the locality matches the option used. Signed-off-by: Andrew Gregory --- src/pacman/conf.h | 2 +- src/pacman/pacman.c | 4 ++-- src/pacman/query.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 45e48c0..c2ed1ca 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -188,7 +188,7 @@ enum { /** package locality */ enum { PKG_LOCALITY_UNSET = 0, - PKG_LOCALITY_LOCAL = (1 << 0), + PKG_LOCALITY_NATIVE = (1 << 0), PKG_LOCALITY_FOREIGN = (1 << 1) }; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 4e91608..578fa99 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -512,11 +512,11 @@ static int parsearg_query(int opt) break; case OP_FOREIGN: case 'm': - config->op_q_locality |= PKG_LOCALITY_LOCAL; + config->op_q_locality |= PKG_LOCALITY_FOREIGN; break; case OP_NATIVE: case 'n': - config->op_q_locality |= PKG_LOCALITY_FOREIGN; + config->op_q_locality |= PKG_LOCALITY_NATIVE; break; case OP_OWNS: case 'o': diff --git a/src/pacman/query.c b/src/pacman/query.c index c9c82b9..5f9f681 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -241,7 +241,7 @@ static unsigned short pkg_get_locality(alpm_pkg_t *pkg) for(j = sync_dbs; j; j = alpm_list_next(j)) { if(alpm_db_get_pkg(j->data, pkgname)) { - return PKG_LOCALITY_LOCAL; + return PKG_LOCALITY_NATIVE; } } return PKG_LOCALITY_FOREIGN; @@ -275,7 +275,7 @@ static int filter(alpm_pkg_t *pkg) return 0; } /* check if this pkg is or isn't in a sync DB */ - if(config->op_q_locality && config->op_q_locality & pkg_get_locality(pkg)) { + if(config->op_q_locality && config->op_q_locality != pkg_get_locality(pkg)) { return 0; } /* check if this pkg is unrequired */ -- 1.8.4

Andrew Gregory

5:32 p.m.

New subject: [pacman-dev] [PATCH 6/7] pacman: die on invalid option combinations

Fixes FS#20950 Signed-off-by: Andrew Gregory --- src/pacman/pacman.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 578fa99..9976a8d 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -323,6 +323,16 @@ static void handler(int signum) #define check_optarg() if(!optarg) { return 1; } +static void invalid_opt(int used, const char *opt1, const char *opt2) +{ + if(used) { + pm_printf(ALPM_LOG_ERROR, + _("invalid option: '%s' and '%s' may not be used together\n"), + opt1, opt2); + cleanup(1); + } +} + static int parsearg_util_addlist(alpm_list_t **list) { char *i, *save; @@ -479,6 +489,13 @@ static int parsearg_database(int opt) return 0; } +static void checkargs_database(void) +{ + invalid_opt(config->flags & ALPM_TRANS_FLAG_ALLDEPS + && config->flags & ALPM_TRANS_FLAG_ALLEXPLICIT, + "--asdeps", "--asexplicit"); +} + static int parsearg_query(int opt) { switch(opt) { @@ -547,6 +564,46 @@ static int parsearg_query(int opt) return 0; } +static void checkargs_query_display_opts(const char *opname) { + invalid_opt(config->op_q_changelog, opname, "--changelog"); + invalid_opt(config->op_q_check, opname, "--check"); + invalid_opt(config->op_q_info, opname, "--info"); + invalid_opt(config->op_q_list, opname, "--list"); +} + +static void checkargs_query_filter_opts(const char *opname) { + invalid_opt(config->op_q_deps, opname, "--deps"); + invalid_opt(config->op_q_explicit, opname, "--explicit"); + invalid_opt(config->op_q_upgrade, opname, "--upgrade"); + invalid_opt(config->op_q_unrequired, opname, "--unrequired"); + invalid_opt(config->op_q_locality & PKG_LOCALITY_NATIVE, opname, "--native"); + invalid_opt(config->op_q_locality & PKG_LOCALITY_FOREIGN, opname, "--foreign"); +} + +static void checkargs_query(void) +{ + if(config->op_q_isfile) { + invalid_opt(config->group, "--file", "--groups"); + invalid_opt(config->op_q_search, "--file", "--search"); + invalid_opt(config->op_q_owns, "--file", "--owns"); + } else if(config->op_q_search) { + invalid_opt(config->group, "--search", "--groups"); + invalid_opt(config->op_q_owns, "--search", "--owns"); + checkargs_query_display_opts("--search"); + checkargs_query_filter_opts("--search"); + } else if(config->op_q_owns) { + invalid_opt(config->group, "--owns", "--groups"); + checkargs_query_display_opts("--owns"); + checkargs_query_filter_opts("--owns"); + } else if(config->group) { + checkargs_query_display_opts("--groups"); + } + + invalid_opt(config->op_q_deps && config->op_q_explicit, "--deps", "--explicit"); + invalid_opt(config->op_q_locality & (PKG_LOCALITY_NATIVE | PKG_LOCALITY_FOREIGN), + "--native", "--foreign"); +} + /* options common to -S -R -U */ static int parsearg_trans(int opt) { @@ -583,6 +640,16 @@ static int parsearg_trans(int opt) return 0; } +static void checkargs_trans(void) +{ + if(config->print) { + invalid_opt(config->flags & ALPM_TRANS_FLAG_DBONLY, + "--print", "--dbonly"); + invalid_opt(config->flags & ALPM_TRANS_FLAG_NOSCRIPTLET, + "--print", "--noscriptlet"); + } +} + static int parsearg_remove(int opt) { if(parsearg_trans(opt) == 0) { @@ -615,6 +682,16 @@ static int parsearg_remove(int opt) return 0; } +static void checkargs_remove(void) +{ + checkargs_trans(); + if(config->flags & ALPM_TRANS_FLAG_NOSAVE) { + invalid_opt(config->print, "--nosave", "--print"); + invalid_opt(config->flags & ALPM_TRANS_FLAG_DBONLY, + "--nosave", "--dbonly"); + } +} + /* options common to -S -U */ static int parsearg_upgrade(int opt) { @@ -637,6 +714,14 @@ static int parsearg_upgrade(int opt) return 0; } +static void checkargs_upgrade(void) +{ + checkargs_trans(); + invalid_opt(config->flags & ALPM_TRANS_FLAG_ALLDEPS + && config->flags & ALPM_TRANS_FLAG_ALLEXPLICIT, + "--asdeps", "--asexplicit"); +} + static int parsearg_sync(int opt) { if(parsearg_upgrade(opt) == 0) { @@ -687,6 +772,38 @@ static int parsearg_sync(int opt) return 0; } +static void checkargs_sync(void) +{ + checkargs_upgrade(); + if(config->op_s_clean) { + invalid_opt(config->group, "--clean", "--groups"); + invalid_opt(config->op_s_info, "--clean", "--info"); + invalid_opt(config->op_q_list, "--clean", "--list"); + invalid_opt(config->op_s_sync, "--clean", "--refresh"); + invalid_opt(config->op_s_search, "--clean", "--search"); + invalid_opt(config->op_s_upgrade, "--clean", "--sysupgrade"); + invalid_opt(config->op_s_downloadonly, "--clean", "--downloadonly"); + } else if(config->op_s_info) { + invalid_opt(config->group, "--info", "--groups"); + invalid_opt(config->op_q_list, "--info", "--list"); + invalid_opt(config->op_s_search, "--info", "--search"); + invalid_opt(config->op_s_upgrade, "--info", "--sysupgrade"); + invalid_opt(config->op_s_downloadonly, "--info", "--downloadonly"); + } else if(config->op_s_search) { + invalid_opt(config->group, "--search", "--groups"); + invalid_opt(config->op_q_list, "--search", "--list"); + invalid_opt(config->op_s_upgrade, "--search", "--sysupgrade"); + invalid_opt(config->op_s_downloadonly, "--search", "--downloadonly"); + } else if(config->op_q_list) { + invalid_opt(config->group, "--list", "--groups"); + invalid_opt(config->op_s_upgrade, "--list", "--sysupgrade"); + invalid_opt(config->op_s_downloadonly, "--list", "--downloadonly"); + } else if(config->group) { + invalid_opt(config->op_s_upgrade, "--groups", "--sysupgrade"); + invalid_opt(config->op_s_downloadonly, "--groups", "--downloadonly"); + } +} + /** Parse command-line arguments for each operation. * @param argc argc * @param argv argv @@ -845,6 +962,29 @@ static int parseargs(int argc, char *argv[]) optind++; } + switch(config->op) { + case PM_OP_DATABASE: + checkargs_database(); + break; + case PM_OP_DEPTEST: + /* no conflicting options */ + break; + case PM_OP_SYNC: + checkargs_sync(); + break; + case PM_OP_QUERY: + checkargs_query(); + break; + case PM_OP_REMOVE: + checkargs_remove(); + break; + case PM_OP_UPGRADE: + checkargs_upgrade(); + break; + default: + break; + } + return 0; } -- 1.8.4

Andrew Gregory

5:32 p.m.

New subject: [pacman-dev] [PATCH 7/7] do not run scriptlets with --dbonly

Running an install script does not fall under "Adds/removes the database entry only." Signed-off-by: Andrew Gregory --- src/pacman/pacman.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 9976a8d..f485692 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -618,6 +618,7 @@ static int parsearg_trans(int opt) break; case OP_DBONLY: config->flags |= ALPM_TRANS_FLAG_DBONLY; + config->flags |= ALPM_TRANS_FLAG_NOSCRIPTLET; break; case OP_NOPROGRESSBAR: config->noprogressbar = 1; -- 1.8.4

Allan McRae

11:55 p.m.

New subject: [pacman-dev] [PATCH 3/7] include invalid options in error messages

On 13/10/13 03:32, Andrew Gregory wrote:

...

On invalid combinations of flags we were only printing the unhelpfully vague message "invalid option".

Signed-off-by: Andrew Gregory --- src/pacman/conf.h | 3 ++- src/pacman/pacman.c | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/pacman/conf.h b/src/pacman/conf.h index e263d7c..45e48c0 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -128,7 +128,8 @@ enum {

/* Long Operations */ enum { - OP_NOCONFIRM = 1000, + OP_LONG_FLAG_MIN = 1000, + OP_NOCONFIRM, OP_CONFIG, OP_IGNORE, OP_DEBUG, diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 93baa44..308ff38 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -828,7 +828,12 @@ static int parseargs(int argc, char *argv[]) result = parsearg_global(opt); if(result != 0) { /* global option parsing failed, abort */ - pm_printf(ALPM_LOG_ERROR, _("invalid option\n")); + if(opt < OP_LONG_FLAG_MIN) { + pm_printf(ALPM_LOG_ERROR, _("invalid option '-%c'\n"), opt); + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid option '--%s'\n"), + opts[option_index].name); + } return result;

I spent some time trying to get to this error. I found tow ways to get there and both do not seem like this is actually the right error message: allan@arya ~

...

pacman --color=foo error: invalid argument 'foo' for --color error: invalid option

allan@arya ~

...

pacman --debug=4 error: '4' is not a valid debug level error: invalid option

How else can we get there?

Allan McRae

13 Oct 13 Oct

12:06 a.m.

New subject: [pacman-dev] [PATCH 3/7] include invalid options in error messages

On 13/10/13 09:55, Allan McRae wrote:

...

On 13/10/13 03:32, Andrew Gregory wrote:

...
On invalid combinations of flags we were only printing the unhelpfully vague message "invalid option".

Signed-off-by: Andrew Gregory --- src/pacman/conf.h | 3 ++- src/pacman/pacman.c | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/pacman/conf.h b/src/pacman/conf.h index e263d7c..45e48c0 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -128,7 +128,8 @@ enum {

/* Long Operations */ enum { - OP_NOCONFIRM = 1000, + OP_LONG_FLAG_MIN = 1000, + OP_NOCONFIRM, OP_CONFIG, OP_IGNORE, OP_DEBUG, diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 93baa44..308ff38 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -828,7 +828,12 @@ static int parseargs(int argc, char *argv[]) result = parsearg_global(opt); if(result != 0) { /* global option parsing failed, abort */ - pm_printf(ALPM_LOG_ERROR, _("invalid option\n")); + if(opt < OP_LONG_FLAG_MIN) { + pm_printf(ALPM_LOG_ERROR, _("invalid option '-%c'\n"), opt); + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid option '--%s'\n"), + opts[option_index].name); + } return result;

I spent some time trying to get to this error. I found tow ways to get there and both do not seem like this is actually the right error message:

allan@arya ~

...
pacman --color=foo error: invalid argument 'foo' for --color error: invalid option

allan@arya ~

...
pacman --debug=4 error: '4' is not a valid debug level error: invalid option

How else can we get there?

Answering my own question:

...

pacman -S --changelog error: invalid option

Andrew Gregory

12:12 a.m.

New subject: [pacman-dev] [PATCH 3/7] include invalid options in error messages

On 10/13/13 at 09:55am, Allan McRae wrote:

...

On 13/10/13 03:32, Andrew Gregory wrote:

...
On invalid combinations of flags we were only printing the unhelpfully vague message "invalid option".

Signed-off-by: Andrew Gregory --- src/pacman/conf.h | 3 ++- src/pacman/pacman.c | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/pacman/conf.h b/src/pacman/conf.h index e263d7c..45e48c0 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -128,7 +128,8 @@ enum {

/* Long Operations */ enum { - OP_NOCONFIRM = 1000, + OP_LONG_FLAG_MIN = 1000, + OP_NOCONFIRM, OP_CONFIG, OP_IGNORE, OP_DEBUG, diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 93baa44..308ff38 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -828,7 +828,12 @@ static int parseargs(int argc, char *argv[]) result = parsearg_global(opt); if(result != 0) { /* global option parsing failed, abort */ - pm_printf(ALPM_LOG_ERROR, _("invalid option\n")); + if(opt < OP_LONG_FLAG_MIN) { + pm_printf(ALPM_LOG_ERROR, _("invalid option '-%c'\n"), opt); + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid option '--%s'\n"), + opts[option_index].name); + } return result;

I spent some time trying to get to this error. I found tow ways to get there and both do not seem like this is actually the right error message:

allan@arya ~

...
pacman --color=foo error: invalid argument 'foo' for --color error: invalid option

allan@arya ~

...
pacman --debug=4 error: '4' is not a valid debug level error: invalid option

How else can we get there?

Use a valid option with an operation that doesn't understand it such as `pacman -T --quiet`.

Allan McRae

12:35 a.m.

New subject: [pacman-dev] [PATCH 3/7] include invalid options in error messages

On 13/10/13 10:12, Andrew Gregory wrote:

...

On 10/13/13 at 09:55am, Allan McRae wrote:

...
On 13/10/13 03:32, Andrew Gregory wrote:

...
On invalid combinations of flags we were only printing the unhelpfully vague message "invalid option".

Signed-off-by: Andrew Gregory --- src/pacman/conf.h | 3 ++- src/pacman/pacman.c | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/pacman/conf.h b/src/pacman/conf.h index e263d7c..45e48c0 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -128,7 +128,8 @@ enum {

/* Long Operations */ enum { - OP_NOCONFIRM = 1000, + OP_LONG_FLAG_MIN = 1000, + OP_NOCONFIRM, OP_CONFIG, OP_IGNORE, OP_DEBUG, diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 93baa44..308ff38 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -828,7 +828,12 @@ static int parseargs(int argc, char *argv[]) result = parsearg_global(opt); if(result != 0) { /* global option parsing failed, abort */ - pm_printf(ALPM_LOG_ERROR, _("invalid option\n")); + if(opt < OP_LONG_FLAG_MIN) { + pm_printf(ALPM_LOG_ERROR, _("invalid option '-%c'\n"), opt); + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid option '--%s'\n"), + opts[option_index].name); + } return result;

I spent some time trying to get to this error. I found tow ways to get there and both do not seem like this is actually the right error message:

allan@arya ~

...
pacman --color=foo error: invalid argument 'foo' for --color error: invalid option

allan@arya ~

...
pacman --debug=4 error: '4' is not a valid debug level error: invalid option

How else can we get there?

Use a valid option with an operation that doesn't understand it such as `pacman -T --quiet`.

I like this patch, so accepted with the note that this output is bad (but was bad in the first place...) allan@arya /home/arch/code/pacman (working)

...

./src/pacman/pacman --color=foo error: invalid argument 'foo' for --color error: invalid option '--color'

3868

Age (days ago)

3869

Last active (days ago)

List overview

Download

10 comments

2 participants

participants (2)

Allan McRae
Andrew Gregory

[pacman-dev] [PATCH 1/7] pacman/util: remove strsplit

tags

participants (2)