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

On Fri, Nov 05, 2010 at 12:42:54AM +1000, Allan McRae wrote:

On 05/11/10 00:20, Dave Reisner 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.

Dan: this sold the patch to me.

...

Revised from the original 7/24 submission at Allan's request to check the return value of freopen().

FYI, comments like this can go below the "---" two lines down. That way they do not get included in the actual git commit.

Cool beans. I should really look at the man page for git-send-email one of these days.

...

Signed-off-by: Dave Reisner --- 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 /* isspace */ #include /* atoi */ #include #include @@ -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)) { + char line[BUFSIZ]; + int i = 0; + while((line[i] = fgetc(stdin)) != EOF) { + if(isspace(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; + } + } + /* 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)) {

Two things here. The HACKING file says to use if(foo() == 0) rather than if(!foo()). But freopen returns a FILE*. So we really should be checking if that == NULL? Also, does the FILE* that freopen returns need closed here or is that fine as is? I get slight confused...

Just above where my patch falls in, is this chunk (line 1275): if(!isatty()) { config->noprogressbar = 1; } The reopened stdin should be closed as usual by the kernel on exit. We only need to check for the presence of a FILE* being returned. When successful, the function just returns the value of the third argument -- stdin in this case.

...

+ pm_printf(PM_LOG_ERROR, _("failed to reopen stdin for reading: (%s)\n"), + strerror(errno)); + } + } + /* parse the config file */ ret = parseconfig(config->configfile); if(ret != 0) {