I'm pretty sure the logic checks out. I've just run it on the command line to check again. found_group increments whenever the group is in any of the databases. If it's in _none_ of the databases (ie you've !found the group) then found_all is permanently set to 1 so that an error will be returned, and it moves onto searching for the next group. If a group is found then found_all isn't touched, and it moves onto searching for the next group.
I think I set found_all to 1 originally because I was going to do something slightly different but I'll initialise it as 0 and rename it to not_found_all when I next update the patch. On 12 July 2013 20:54, Simon Gomizelj <[email protected]> wrote: > On Fri, Jul 12, 2013 at 04:56:32PM +0100, [email protected] > wrote: >> From: Ashley Whetter <[email protected]> >> >> Non-zero is now returned if a group is searched for that doesn't exist >> >> Signed-off-by: Ashley Whetter <[email protected]> >> --- >> 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..35a5c59 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_group = 0, found_all = 1; >> >> if(targets) { >> 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_group++; >> /* get names of packages in group */ >> for(k = grp->packages; k; k = >> alpm_list_next(k)) { >> if(!config->quiet) { >> @@ -387,6 +389,11 @@ static int sync_group(int level, alpm_list_t *syncs, >> alpm_list_t *targets) >> } >> } >> } >> + >> + if (!found_group) { >> + found_all = 0; >> + } >> + found_group = 0; >> } > > Unless I misunderstood something, I don't think this is the logic you > described. You're counting each time a group is found in a given db, and > then checking that the count is non-zero. Thus you'll only trigger > a return code of 1 if _none_ of your searches result in a match. > > It should return 1 if any one search fails. > >> } else { >> for(i = syncs; i; i = alpm_list_next(i)) { >> @@ -394,6 +401,7 @@ static int sync_group(int level, alpm_list_t *syncs, >> alpm_list_t *targets) >> >> for(j = alpm_db_get_groupcache(db); j; j = >> alpm_list_next(j)) { >> alpm_group_t *grp = j->data; >> + found_all = 1; > > found_all is already set to one. This else block is completely > independent. > >> - return 0; >> + return !found_all; > > The negation here seems arbitrary and needless. You're explicitly setting > found_all to either 1 or 0 in the first place. >
