[pacman-dev] Fix for FS#36097

[Dave Reisner]

On Thu, Jul 11, 2013 at 11:42:38PM +0100, Ashley Whetter wrote:
> Hey everyone,
> 
> I've attached (and pasted below) a couple of possible patches that fix
> FS#36097 (Wrong exit code for `pacman -Sg`).

Please start using git-send-email in the future if you expect these to
be reviewed and applied properly:

https://www.archlinux.org/pacman/submitting-patches.html

> From 045b58ada05747c6743e8bf68e7e7d8156016eee Mon Sep 17 00:00:00 2001
> From: Ashley Whetter <awhetter.2011 at my.bristol.ac.uk>
> Date: Fri, 12 Jul 2013 00:15:15 +0100
> Subject: [PATCH] Sg flag now returns correct return value for errors
> 
> sync_group now returns non-zero if no groups are found
>         in the (non-empty) list of groups to search for
> 
> Fixes FS#36097
> 
> Signed-off-by: Ashley Whetter <awhetter.2011 at my.bristol.ac.uk>
> ---
>  src/pacman/sync.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index fc1314b..af26ae4 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;
> 
>   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 = 1;
>   /* get names of packages in group */
>   for(k = grp->packages; k; k = alpm_list_next(k)) {
>   if(!config->quiet) {
> @@ -396,6 +398,8 @@ static int sync_group(int level, alpm_list_t
> *syncs, alpm_list_t *targets)
>   alpm_group_t *grp = j->data;
> 
>   if(level > 1) {
> + found_group |= (int)(intptr_t) grp->packages;

This cast doesn't buy you anything. Just compare the pointer to NULL...

> +
>   for(k = grp->packages; k; k = alpm_list_next(k)) {
>   printf("%s %s\n", grp->name,
>   alpm_pkg_get_name(k->data));
> @@ -403,6 +407,7 @@ static int sync_group(int level, alpm_list_t
> *syncs, alpm_list_t *targets)
>   } else {
>   /* print grp names only, no package names */
>   if(!alpm_list_find_str (s, grp->name)) {
> + found_group = 1;
>   s = alpm_list_add (s, grp->name);
>   printf("%s\n", grp->name);
>   }
> @@ -412,7 +417,7 @@ static int sync_group(int level, alpm_list_t
> *syncs, alpm_list_t *targets)
>   alpm_list_free(s);
>   }
> 
> - return 0;
> + return (found_group == 0);
>  }
> 
>  static int sync_info(alpm_list_t *syncs, alpm_list_t *targets)
> --
> 1.8.3.2
> 
> From 2b36585f5aeca8adbe15fc4609ff4bc5536af798 Mon Sep 17 00:00:00 2001
> From: Ashley Whetter <awhetter.2011 at my.bristol.ac.uk>
> Date: Fri, 12 Jul 2013 00:35:55 +0100
> Subject: [PATCH] Sg flag now returns correct return value for errors
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> sync_group now returns non-zero if no groups are found
>         in the (non-empty) list of groups to search for
> 
> Fixes FS#36097
> 
> Signed-off-by: Ashley Whetter <awhetter.2011 at my.bristol.ac.uk>
> ---
>  src/pacman/sync.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index fc1314b..9d2dbd6 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 = 0;
> 
>   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 = 1;
>   /* get names of packages in group */
>   for(k = grp->packages; k; k = alpm_list_next(k)) {
>   if(!config->quiet) {
> @@ -410,9 +412,13 @@ static int sync_group(int level, alpm_list_t
> *syncs, alpm_list_t *targets)
>   }
>   }
>   alpm_list_free(s);
> +
> + /* Even if there are no groups we have still done everything we should
> + * have */
> + found = 1;
>   }
> 
> - return 0;
> + return (found == 0);
>  }
> 
>  static int sync_info(alpm_list_t *syncs, alpm_list_t *targets)
> --
> 1.8.3.2
> 
> 
> With the first patch, the new double cast line is a bit weird but
> genuinely flags whether or not we found any groups.
> The second patch seems a bit more sane to me in terms of the
> functionality. It just marks there to be a group found if the user
> didn't search for a specific group, regardless of whether any groups
> were actually found. I think this makes sense because in the highly
> unlikely scenario that there are no groups and the user didn't search
> for any specifically, pacman has still done everything it should have
> done so shouldn't return an error.
> 
> Regards,
> Ashley



>