Re: [pacman-dev] [PATCH v2] Corrected return codes for Sg flag

On 14/07/13 02:01, awhetter.2011@my.bristol.ac.uk wrote:

From: Ashley Whetter

Non-zero is now returned if a group is searched for that doesn't exist Fixes FS#36097

Signed-off-by: Ashley Whetter --- In this version I've renamed the variables as Simon suggested. Also I noticed that when no targets were specified the behaviour of -Ss wasn't mirrored and a success would always be returned. So I've have to reinitialise 'found' if there are no targets, which isn't ideal but the only other decent options I could think of was to either name 'found' as 'not_found' and flip the 0s and 1s accordingly, or to check the length of alpm_db_get_groupcache(db) by taking the 'j =' initiasation out of the for loop, which seemed worse. Also the way I'm returning 'found' is inconsistent with how it's done in sync_search, so I feel like sync_search should be changed as well because '!found' is more readable than '(found == 0)'. src/pacman/sync.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/pacman/sync.c b/src/pacman/sync.c index fc1314b..313c3cd 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -367,6 +367,7 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets) { alpm_list_t *i, *j, *k, *s = NULL; + int found_in_db = 0, found = 1;

I'm going to ask for a change here, which is mostly just a variable renaming... but requires other minor adjustment. The "found" variable is really just tracking our return value. So name it that. Also "found_in_db" makes me concerned that things can be found outside a db! So: found_in_db -> found found -> ret

if(targets) {

found_in_db (-> found) is only used in this scope so define it here.

for(i = targets; i; i = alpm_list_next(i)) { @@ -376,6 +377,7 @@ static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets) alpm_group_t *grp = alpm_db_get_group(db, grpname);

if(grp) { + found_in_db++; /* get names of packages in group */ for(k = grp->packages; k; k = alpm_list_next(k)) { if(!config->quiet) { @@ -387,13 +389,19 @@ static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets) } } } + if (!found_in_db) { + found = 0; + } + found_in_db = 0;

Set this at the start of the loop.

} } else { + found = 0; for(i = syncs; i; i = alpm_list_next(i)) { alpm_db_t *db = i->data;

for(j = alpm_db_get_groupcache(db); j; j = alpm_list_next(j)) { alpm_group_t *grp = j->data; + found = 1;

if(level > 1) { for(k = grp->packages; k; k = alpm_list_next(k)) { @@ -412,7 +420,7 @@ static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets) alpm_list_free(s); }

- return 0; + return !found; }

static int sync_info(alpm_list_t *syncs, alpm_list_t *targets)