[pacman-dev] [PATCH] Support reading package args from stdin

[Dan McGee]

On Thu, Nov 4, 2010 at 9:20 AM, Dave Reisner <d at falconindy.com> wrote:
> Only occurs if no arguments were provided directly. Arguments can be
> separated by any amount of valid whitespace. This allows for piping into
> pacman from other programs or from itself, e.g.:
>
>  pacman -Qdtq | pacman -Rs
>
> This is better than using xargs, as xargs will not reconnect stdin to
> the terminal. The above operation performed using xargs would require
> the --noconfirm flag to be passed to pacman.
>
> Revised from the original 7/24 submission at Allan's request to check
> the return value of freopen().
>
> Signed-off-by: Dave Reisner <d at falconindy.com>
> ---
>  src/pacman/pacman.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index 15abecc..6f40a97 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -26,6 +26,7 @@
>  #define PACKAGE_VERSION GIT_VERSION
>  #endif
>
> +#include <ctype.h> /* isspace */
>  #include <stdlib.h> /* atoi */
>  #include <stdio.h>
>  #include <limits.h>
> @@ -1305,6 +1306,32 @@ int main(int argc, char *argv[])
>                cleanup(ret);
>        }
>
> +       /* read package arguments from stdin if we have none yet */
> +       if(!pm_targets && !isatty(0)) {
isatty(fileno(stdin)) is technically more correct, no? It also would
handle the case where someone reassigned stdin.

> +               char line[BUFSIZ];
Another constant for a buffer size? If there is one in our codebase
I'd rather we use that, and of course it would be even better to not
have an arbitrary limit on this at all. See later comment.

> +               int i = 0;
> +               while((line[i] = fgetc(stdin)) != EOF) {
> +                       if(isspace(line[i])) {
> +                               line[i] = 0;
Use '\0' please, this is a char, not an int.

> +                               /* avoid adding zero length arg when multiple spaces separate args */
> +                               if(i > 0) {
> +                                       pm_targets = alpm_list_add(pm_targets, strdup(line));
> +                                       i = 0;
> +                               }
> +                       } else {
> +                               ++i;
i++; we don't use the pre-increment operator anywhere and it makes no
difference so we should style-match.

> +                       }
> +               }
Uhhhhh- buffer overflow condition?

> +               /* end of stream -- check for data still in line buffer */
> +               if(i > 0) {
> +                       pm_targets = alpm_list_add(pm_targets, strdup(line));
> +               }
> +               if (!freopen(ctermid(NULL), "r", stdin)) {
> +                       pm_printf(PM_LOG_ERROR, _("failed to reopen stdin for reading: (%s)\n"),
> +                                       strerror(errno));
> +               }
> +       }
> +
So the other thing I'm thinking here is it breaks anyone that might
have piped into pacman before. I'm not sure if we care, but using
/usr/bin/yes did work before. Is it worth guarding this functionality
behind an option (--use-stdin or something) instead of the isatty()
check (which would take place on the freopen() call instead)?

>        /* parse the config file */
>        ret = parseconfig(config->configfile);
>        if(ret != 0) {
> --
> 1.7.3.2
>
>
>