[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