[pacman-dev] [PATCH 5/6v2] allow specifying input to scriptlets

Allan McRae allan at archlinux.org
Wed Nov 11 05:01:17 UTC 2015


On 27/10/15 14:47, Andrew Gregory wrote:
> Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> ---
>  lib/libalpm/hook.c  |   2 +-
>  lib/libalpm/trans.c |   2 +-
>  lib/libalpm/util.c  | 174 ++++++++++++++++++++++++++++++++++++++++++++--------
>  lib/libalpm/util.h  |   5 +-
>  4 files changed, 154 insertions(+), 29 deletions(-)
> 
> diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
> index 45c10e1..b449ce0 100644
> --- a/lib/libalpm/hook.c
> +++ b/lib/libalpm/hook.c
> @@ -511,7 +511,7 @@ static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook)
>  		}
>  	}
>  
> -	return _alpm_run_chroot(handle, hook->cmd[0], hook->cmd);
> +	return _alpm_run_chroot(handle, hook->cmd[0], hook->cmd, NULL, NULL);
>  }
>  
>  int _alpm_hook_run(alpm_handle_t *handle, enum _alpm_hook_when_t when)

OK.

> diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
> index a6b1aef..06997a0 100644
> --- a/lib/libalpm/trans.c
> +++ b/lib/libalpm/trans.c
> @@ -391,7 +391,7 @@ int _alpm_runscriptlet(alpm_handle_t *handle, const char *filepath,
>  
>  	_alpm_log(handle, ALPM_LOG_DEBUG, "executing \"%s\"\n", cmdline);
>  
> -	retval = _alpm_run_chroot(handle, SCRIPTLET_SHELL, argv);
> +	retval = _alpm_run_chroot(handle, SCRIPTLET_SHELL, argv, NULL, NULL);
>  
>  cleanup:
>  	if(scriptfn && unlink(scriptfn)) {

OK.

> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index 66a2742..eed0293 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -31,6 +31,7 @@
>  #include <limits.h>
>  #include <sys/wait.h>
>  #include <fnmatch.h>
> +#include <poll.h>
>  
>  /* libarchive */
>  #include <archive.h>
> @@ -445,16 +446,40 @@ ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path,
>  	return files;
>  }
>  
> +/* write wrapper that ignores SIGPIPE */

write() wrapper ...

Also, this comment is worth expanding - why do we need this?

> +static ssize_t _alpm_pipe_write(int fd, const void *buf, size_t count)
> +{
> +	sigset_t new, old;
> +	ssize_t ret;
> +	sigemptyset(&new);
> +	sigaddset(&new, SIGPIPE);
> +	pthread_sigmask(SIG_BLOCK, &new, &old);
> +	ret = write(fd, buf, count);
> +	pthread_sigmask(SIG_SETMASK, &old, NULL);
> +	return ret;
> +}
> +

Why not use signal(SIGPIPE, SIG_IGN)?

> +static void _alpm_handle_script_output(alpm_handle_t *handle, const char *line)
> +{
> +	alpm_event_scriptlet_info_t event = {
> +		.type = ALPM_EVENT_SCRIPTLET_INFO,
> +		.line = line
> +	};
> +	alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line);
> +	EVENT(handle, &event);
> +}
> +
>  /** Execute a command with arguments in a chroot.
>   * @param handle the context handle
>   * @param cmd command to execute
>   * @param argv arguments to pass to cmd

Needs updated.

>   * @return 0 on success, 1 on error
>   */
> -int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[])
> +int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[],
> +		_alpm_cb_io p2c_cb, void *p2c_ctx)

void *...   can this be alpm_list_t?

>  {
>  	pid_t pid;
> -	int pipefd[2], cwdfd;
> +	int c2p_pipefd[2], p2c_pipefd[2], cwdfd;

child2parent, parent2child  - I would like these names expanded.

>  	int retval = 0;
>  
>  	/* save the cwd so we can restore it later */
> @@ -476,7 +501,13 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[])
>  	/* Flush open fds before fork() to avoid cloning buffers */
>  	fflush(NULL);
>  
> -	if(pipe(pipefd) == -1) {
> +	if(pipe(c2p_pipefd) == -1) {
> +		_alpm_log(handle, ALPM_LOG_ERROR, _("could not create pipe (%s)\n"), strerror(errno));
> +		retval = 1;
> +		goto cleanup;
> +	}
> +
> +	if(p2c_cb && pipe(p2c_pipefd) == -1) {
>  		_alpm_log(handle, ALPM_LOG_ERROR, _("could not create pipe (%s)\n"), strerror(errno));
>  		retval = 1;
>  		goto cleanup;

OK

> @@ -495,10 +526,15 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[])
>  		close(0);
>  		close(1);
>  		close(2);
> -		while(dup2(pipefd[1], 1) == -1 && errno == EINTR);
> -		while(dup2(pipefd[1], 2) == -1 && errno == EINTR);
> -		close(pipefd[0]);
> -		close(pipefd[1]);
> +		while(dup2(c2p_pipefd[1], 1) == -1 && errno == EINTR);
> +		while(dup2(c2p_pipefd[1], 2) == -1 && errno == EINTR);
> +		if(p2c_cb) {
> +			while(dup2(p2c_pipefd[0], 0) == -1 && errno == EINTR);
> +			close(p2c_pipefd[0]);
> +			close(p2c_pipefd[1]);
> +		}
> +		close(c2p_pipefd[0]);
> +		close(c2p_pipefd[1]);
>  		if(cwdfd >= 0) {
>  			close(cwdfd);
>  		}

OK

> @@ -521,29 +557,115 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[])
>  	} else {
>  		/* this code runs for the parent only (wait on the child) */
>  		int status;
> -		FILE *pipe_file;
> -
> -		close(pipefd[1]);
> -		pipe_file = fdopen(pipefd[0], "r");
> -		if(pipe_file == NULL) {
> -			close(pipefd[0]);
> -			retval = 1;
> +		char obuf[PIPE_BUF]; /* writes <= PIPE_BUF are guaranteed atomic */
> +		char ibuf[LINE_MAX + 2]; /* +2 for appending \n\0 */
> +		ssize_t olen = 0, ilen = 0;
> +		struct pollfd fds[2], *c2p = &(fds[0]), *p2c = &(fds[1]);
> +		nfds_t nfds = 2;
> +
> +		c2p->fd = c2p_pipefd[0];
> +		c2p->events = POLLIN;
> +		fcntl(c2p->fd, F_SETFL, O_NONBLOCK);
> +		close(c2p_pipefd[1]);
> +

OK

> +		if(p2c_cb) {
> +			p2c->fd = p2c_pipefd[1];
> +			p2c->events = POLLOUT;
> +			fcntl(p2c->fd, F_SETFL, O_NONBLOCK);
> +			close(p2c_pipefd[0]);
>  		} else {
> -			while(!feof(pipe_file)) {
> -				char line[PATH_MAX];
> -				alpm_event_scriptlet_info_t event = {
> -					.type = ALPM_EVENT_SCRIPTLET_INFO,
> -					.line = line
> -				};
> -				if(safe_fgets(line, PATH_MAX, pipe_file) == NULL) {
> -					break;
> +			p2c->fd = -1;
> +			p2c->events = 0;
> +		}

OK


And, I am lost from here...

> +
> +#define STOP_POLLING(p) do { close(p->fd); p->fd = -1; } while(0)
> +
> +		while((c2p->fd != -1 || p2c->fd != -1) && poll(fds, nfds, -1) > 0) {
> +			if(c2p->revents & POLLIN) {
> +				ssize_t space = LINE_MAX - ilen;
> +				ssize_t nread = read(c2p->fd, ibuf + ilen, space);
> +				if(nread > 0) {
> +					char *newline = memchr(ibuf + ilen, '\n', nread);
> +					ilen += nread;
> +					if(newline) {
> +						while(newline) {
> +							size_t linelen = newline - ibuf + 1;
> +							char old = ibuf[linelen];
> +							ibuf[linelen] = '\0';
> +							_alpm_handle_script_output(handle, ibuf);
> +							ibuf[linelen] = old;
> +
> +							ilen -= linelen;
> +							memmove(ibuf, ibuf + linelen, ilen);
> +							newline = memchr(ibuf, '\n', ilen);
> +						}
> +					} else if(nread == space) {
> +						/* we didn't read a full line, but we're out of space */
> +						strcpy(ibuf + ilen, "\n");
> +						_alpm_handle_script_output(handle, ibuf);
> +						ilen = 0;
> +					}
> +				} else if(nread == 0) {
> +					/* end-of-file */
> +					if(ilen) {
> +						strcpy(ibuf + ilen, "\n");
> +						_alpm_handle_script_output(handle, ibuf);
> +					}
> +					STOP_POLLING(c2p);
> +				} else if(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
> +					/* nothing read, try again */
> +				} else {
> +					/* read error  */
> +					if(ilen) {
> +						strcpy(ibuf + ilen, "\n");
> +						_alpm_handle_script_output(handle, ibuf);
> +					}
> +					_alpm_log(handle, ALPM_LOG_ERROR,
> +							_("unable to read from pipe (%s)\n"), strerror(errno));
> +					STOP_POLLING(c2p);
> +				}
> +			} else if(c2p->revents) {
> +				/* anything but POLLIN indicates an error */
> +				STOP_POLLING(c2p);
> +			}
> +			if(p2c->revents & POLLOUT) {
> +				ssize_t nwrite;
> +				if(olen == 0) {
> +					/* empty buffer, ask the callback for more */
> +					if((olen = p2c_cb(obuf, PIPE_BUF, p2c_ctx)) == 0) {
> +						/* no more to write, close the pipe */
> +						STOP_POLLING(p2c);
> +						continue;
> +					}
> +				}
> +				if((nwrite = _alpm_pipe_write(p2c->fd, obuf, olen)) != -1) {
> +					olen -= nwrite;
> +					memmove(obuf, obuf + nwrite, olen);
> +				} else {
> +					if(errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
> +						/* nothing written, try again later */
> +					} else {
> +						/* something went wrong, close the pipe */
> +						_alpm_log(handle, ALPM_LOG_ERROR,
> +								_("unable to write to pipe (%s)\n"), strerror(errno));
> +						STOP_POLLING(p2c);
> +					}
>  				}
> -				alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line);
> -				EVENT(handle, &event);
> +			} else if(p2c->revents) {
> +				/* anything but POLLOUT indicates an error */
> +				STOP_POLLING(p2c);
>  			}
> -			fclose(pipe_file);
>  		}
>  

... to here.

This has gone from being a large function to a massive one - is there
any way to split it up into smaller components?


> +		if(p2c->fd != -1) {
> +			close(p2c->fd);
> +		}
> +		if(c2p->fd != -1) {
> +			close(c2p->fd);
> +		}
> +
> +#undef STOP_POLLING
> +

OK

>  		while(waitpid(pid, &status, 0) == -1) {
>  			if(errno != EINTR) {
>  				_alpm_log(handle, ALPM_LOG_ERROR, _("call to waitpid failed (%s)\n"), strerror(errno));
> @@ -605,7 +727,7 @@ int _alpm_ldconfig(alpm_handle_t *handle)
>  			char arg0[32];
>  			char *argv[] = { arg0, NULL };
>  			strcpy(arg0, "ldconfig");
> -			return _alpm_run_chroot(handle, LDCONFIG, argv);
> +			return _alpm_run_chroot(handle, LDCONFIG, argv, NULL, NULL);
>  		}
>  	}
>  

OK

> diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
> index 95112cf..c0c9ff0 100644
> --- a/lib/libalpm/util.h
> +++ b/lib/libalpm/util.h
> @@ -119,7 +119,10 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix,
>  
>  ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path, int full_count);
>  
> -int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]);
> +typedef ssize_t (*_alpm_cb_io)(void *buf, ssize_t len, void *ctx);
> +
> +int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[],
> +		_alpm_cb_io in_cb, void *in_ctx);
>  int _alpm_ldconfig(alpm_handle_t *handle);
>  int _alpm_str_cmp(const void *s1, const void *s2);
>  char *_alpm_filecache_find(alpm_handle_t *handle, const char *filename);
> 


More information about the pacman-dev mailing list