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

[Dan McGee]

On Thu, Nov 4, 2010 at 3:05 PM, 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.
>
> Signed-off-by: Dave Reisner <d at falconindy.com>
> ---
> Resending as per Allan's request with modifications from the origin 7/24
> submission.
>
> Changes:
> * Check return value of freopen
> * Assign '\0' instead of 0 to null terminate
> * Use post-increment instead of pre-increment
> * Check for buffer overflow condition (not convinced my method is reasonable)
> * Dynamically find stdin's FD instead of assuming it's 1
> * Use PATH_MAX instead of BUFSIZ
>
> Concerns:
> * The verbiage I've used in the new strings is probably not ideal.
>
>  src/pacman/pacman.c |   33 +++++++++++++++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index 15abecc..e078aab 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,38 @@ int main(int argc, char *argv[])
>                cleanup(ret);
>        }
>
> +       /* read package arguments from stdin if we have none yet */
> +       if(!pm_targets && !isatty(fileno(stdin))) {
We talked on IRC, but probably can kill the pm_targets check- why
limit people if we are going to allow this?

> +               char line[PATH_MAX];
> +               int i = 0;
> +               while(i < PATH_MAX && (line[i] = fgetc(stdin)) != EOF) {
> +                       if(isspace((unsigned char)line[i])) {
> +                               line[i] = '\0';
> +                               /* 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++;
> +                       }
> +               }
> +               /* check for buffer overflow */
> +               if (i >= PATH_MAX) {
> +                       pm_printf(PM_LOG_ERROR, _("buffer overflow detected in arg parsing\n"));
How about "could not process stdin: argument greater than %d
characters\n" or something (obv passing PATH_MAX along to the printf
function)?

> +                       cleanup(EXIT_FAILURE);
> +               }
> +
> +               /* end of stream -- check for data still in line buffer */
> +               if(i > 0) {
Do we know line is going to end in a ptr to a null char? Or do you
have to do that here too like you did above.

> +                       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));
If it is a true error, should we cleanup() here too? Otherwise make
this a PM_LOG_WARN probably.

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

Otherwise, this + documentation in pacman man page would be kickass
and ready to commit.

-Dan