On 14/07/13 02:01, [email protected] wrote:
> From: Ashley Whetter <[email protected]>
>
> Non-zero is now returned if a group is searched for that doesn't exist
> Fixes FS#36097
>
> Signed-off-by: Ashley Whetter <[email protected]>
> ---
> 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)
>
