Omar Polo: 4 drop wordexp drop wordexp drop wordexp drop wordexp 6 files changed, 501 insertions(+), 264 deletions(-)
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~exec64/imv-devel/patches/33802/mbox | git am -3Learn more about email & git
Hello, OpenBSD libc doesn't provide wordexp(3), and this is on purpose. This is an attempt at getting rid of it, either by manually constructing strings, using glob(3) or explicitly spawning a subshell. I think that this is an improvement after all. get_config_path was a TOCTOU, the only way to know if a file is readable is to open(2) it; between the access(3) call and open(2) everything can happen. The code got a bit longer but maybe there are ways to shorten it? command_open was changed to use glob(3). This means that command substitution is not done (i.e. :open $(foobar) doesn't work -- is anyone really doing something like that?) but on the other hand it got probably faster since it doesn't need to spawn a shell. The GLOB_BRACE and GLOB_TILDE are actually extensions to the standard but seems widely available so I kept them. (At least on OpenBSD I need to either unset _XOPEN_SOURCE or define _BSD_SOURCE; don't know if this is needed on other systems too. for the time being i haven't changed the meson.build file) generate_env_text instead shouldn't be changed: basically I've inlined parts of the musl implementation of wordexp.c so it should behave as before. (white lie: as a bonus I'm deleting the trailing whitespaces tho) With this both the 4.3.1 release and the latest master branch works out of the box on OpenBSD -CURRENT :) Truth to be told, generate_env_text is causing some issues here. It's called repeatedly and every time it needs to spawn a subshell: it's not difficult with a couple of other applications open to hit the ulimits and see fork(2) failing when viewing a gif for example. (maybe running a subshell per frame is a bit too much?) This is however a separate issue and I'll be happy to open a separate ticket for it. Thanks! Omar Polo diff --git a/src/imv.c b/src/imv.c index 8380a12..edee4cb 100644 --- a/src/imv.c +++ b/src/imv.c @@ -4,6 +4,7 @@ #include <ctype.h> #include <errno.h> #include <getopt.h> +#include <glob.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -11,7 +12,6 @@ #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <wordexp.h> #include "backend.h" #include "binds.h" @@ -1417,40 +1417,6 @@ static void render_window(struct imv *imv) imv->cache_invalidated = false; } -static char *get_config_path(void) -{ - const char *config_paths[] = { - "$imv_config", - "$XDG_CONFIG_HOME/imv/config", - "$HOME/.config/imv/config", - "$HOME/.imv_config", - "$HOME/.imv/config", - "/usr/local/etc/imv_config", - "/etc/imv_config", - }; - - for (size_t i = 0; i < sizeof(config_paths) / sizeof(char*); ++i) { - wordexp_t word; - if (wordexp(config_paths[i], &word, 0) == 0) { - if (!word.we_wordv[0]) { - wordfree(&word); - continue; - } - - char *path = strdup(word.we_wordv[0]); - wordfree(&word); - - if (!path || access(path, R_OK) == -1) { - free(path); - continue; - } - - return path; - } - } - return NULL; -} - static bool parse_bool(const char *str) { return ( @@ -1611,26 +1577,80 @@ static int handle_ini_value(void *user, const char *section, const char *name, return 0; } +static int load_config(const char *path, struct imv *imv) +{ + int err; + + err = ini_parse(path, handle_ini_value, imv); + if (err > 0) + imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); + return err; +} + bool imv_load_config(struct imv *imv) { - char *path = get_config_path(); - if (!path) { - /* no config, no problem - we have defaults */ - return true; + char p[PATH_MAX]; + char *t; + int err; + + if ((t = getenv("imv_config")) != NULL) { + err = load_config(t, imv); + if (err > 0) + return false; + if (err == 0) + return true; } - bool result = true; + if ((t = getenv("XDG_CONFIG_HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + } - const int err = ini_parse(path, handle_ini_value, imv); - if (err == -1) { - imv_log(IMV_ERROR, "Unable to open config file: %s\n", path); - result = false; - } else if (err > 0) { - imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); - result = false; + if ((t = getenv("HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/.config/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv_config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } } - free(path); - return result; + + err = load_config("/usr/local/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + err = load_config("/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + /* no config, no problem - we have defaults */ + return true; } static void command_quit(struct list *args, const char *argstr, void *data) @@ -1760,12 +1780,13 @@ static void command_open(struct list *args, const char *argstr, void *data) continue; } - wordexp_t word; - if (wordexp(args->items[i], &word, 0) == 0) { - for (size_t j = 0; j < word.we_wordc; ++j) { - imv_navigator_add(imv->navigator, word.we_wordv[j], recursive); + glob_t g; + if (glob(args->items[i], GLOB_BRACE|GLOB_TILDE, NULL, &g) == 0) { + char **matches; + for (matches = g.gl_pathv; *matches; ++matches) { + imv_navigator_add(imv->navigator, *matches, recursive); } - wordfree(&word); + globfree(&g); } } } @@ -1995,22 +2016,57 @@ static void update_env_vars(struct imv *imv) static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, const char *format) { + size_t len = 0; + pid_t pid; + int p[2]; /* read end, write end */ + + assert(buf_len > 0); + update_env_vars(imv); - size_t len = 0; - wordexp_t word; - setenv("IFS", "", 1); - if (wordexp(format, &word, 0) == 0) { - for (size_t i = 0; i < word.we_wordc; ++i) { - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]); - } - wordfree(&word); - } else { - len += snprintf(buf, buf_len, "error expanding text"); + if (pipe(p) == -1) + goto err; + + switch (pid = fork()) { + case -1: + close(p[0]); + close(p[1]); + goto err; + case 0: /* child */ + close(p[0]); + setenv("IFS", "", 1); + if (dup2(p[1], 1) != -1) { + execl("/bin/sh", "sh", "-c", "eval \"printf '%s ' $1\"", "sh", + format, (char *)NULL); + } + _exit(1); + default: /* parent */ + close(p[1]); + + size_t r, siz = buf_len - 1, tot = 0; + do { + r = read(p[0], buf, siz); + if (r == -1) { + goto err; + } else if (r == 0) { + break; + } + tot += r; + buf += r; + siz -= r; + } while (siz > 0); + + buf[tot] = '\0'; + while (tot > 0 && isspace((unsigned char)buf[tot-1])) + buf[--tot] = '\0'; + + close(p[0]); + return tot > 0 ? tot-1 : tot; /* don't count the NUL terminator */ } - unsetenv("IFS"); - return len; +err: + snprintf(buf, buf_len, "error expanding text: %s", strerror(errno)); + return strlen(buf); } static size_t read_from_stdin(void **buffer)
builds.sr.ht <builds@sr.ht>imv/patches: FAILED in 3m57s [drop wordexp][0] from [Omar Polo][1] [0]: https://lists.sr.ht/~exec64/imv-devel/patches/33802 [1]: mailto:op@omarpolo.com ✓ #800161 SUCCESS imv/patches/debian.yml https://builds.sr.ht/~exec64/job/800161 ✗ #800158 FAILED imv/patches/freebsd.yml https://builds.sr.ht/~exec64/job/800158 ✗ #800159 FAILED imv/patches/ubuntu.yml https://builds.sr.ht/~exec64/job/800159 ✗ #800160 FAILED imv/patches/archlinux.yml https://builds.sr.ht/~exec64/job/800160
woop, I've left by accident the unused variable `len'. GLOB_BRACE and GLOB_TILDE fails on other systems due to _XOPEN_SOURCE being defined. For FreeBSD defining _BSD_SOURCE too should do it (it's required on OpenBSD too), for glibc I have no idea. _GNU_SOURCE maybe? (or maybe drop _XOPEN_SOURCE.) Here's an updated diff without the unused variable.
Omar Polo <op@omarpolo.com>The previous diff lacked a wait(2) call to reap the child process spawned by generate_env_text. I'm also adding -D_BSD_SOURCE to get the GLOB_* defines. The reason I hit the limits mentioned in the first mail was due to the lack of the wait(2) call; now imv seems to work perfectly fine :)Omar Polo <op@omarpolo.com>...aaand i can't type it seems. There was a typo in the waitpid loop, it should have been (w == -1) and not != -1. sorry for the noise. diff --git a/meson.build b/meson.build index f5568de..4c413f3 100644 --- a/meson.build +++ b/meson.build @@ -18,6 +18,7 @@ endif add_project_arguments('-DIMV_VERSION="@0@"'.format(version), language: 'c') add_project_arguments('-D_XOPEN_SOURCE=700', language: 'c') +add_project_arguments('-D_BSD_SOURCE', language: 'c') add_project_arguments('-DRSVG_DISABLE_DEPRECATION_WARNINGS', language: 'c') diff --git a/src/imv.c b/src/imv.c index 8380a12..6db9cc8 100644 --- a/src/imv.c +++ b/src/imv.c @@ -1,9 +1,12 @@ #include "imv.h" +#include <sys/wait.h> + #include <assert.h> #include <ctype.h> #include <errno.h> #include <getopt.h> +#include <glob.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -11,7 +14,6 @@ #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <wordexp.h> #include "backend.h" #include "binds.h" @@ -1417,40 +1419,6 @@ static void render_window(struct imv *imv) imv->cache_invalidated = false; } -static char *get_config_path(void) -{ - const char *config_paths[] = { - "$imv_config", - "$XDG_CONFIG_HOME/imv/config", - "$HOME/.config/imv/config", - "$HOME/.imv_config", - "$HOME/.imv/config", - "/usr/local/etc/imv_config", - "/etc/imv_config", - }; - - for (size_t i = 0; i < sizeof(config_paths) / sizeof(char*); ++i) { - wordexp_t word; - if (wordexp(config_paths[i], &word, 0) == 0) { - if (!word.we_wordv[0]) { - wordfree(&word); - continue; - } - - char *path = strdup(word.we_wordv[0]); - wordfree(&word); - - if (!path || access(path, R_OK) == -1) { - free(path); - continue; - } - - return path; - } - } - return NULL; -} - static bool parse_bool(const char *str) { return ( @@ -1611,26 +1579,80 @@ static int handle_ini_value(void *user, const char *section, const char *name, return 0; } +static int load_config(const char *path, struct imv *imv) +{ + int err; + + err = ini_parse(path, handle_ini_value, imv); + if (err > 0) + imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); + return err; +} + bool imv_load_config(struct imv *imv) { - char *path = get_config_path(); - if (!path) { - /* no config, no problem - we have defaults */ - return true; + char p[PATH_MAX]; + char *t; + int err; + + if ((t = getenv("imv_config")) != NULL) { + err = load_config(t, imv); + if (err > 0) + return false; + if (err == 0) + return true; } - bool result = true; + if ((t = getenv("XDG_CONFIG_HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + } - const int err = ini_parse(path, handle_ini_value, imv); - if (err == -1) { - imv_log(IMV_ERROR, "Unable to open config file: %s\n", path); - result = false; - } else if (err > 0) { - imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); - result = false; + if ((t = getenv("HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/.config/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv_config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } } - free(path); - return result; + + err = load_config("/usr/local/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + err = load_config("/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + /* no config, no problem - we have defaults */ + return true; } static void command_quit(struct list *args, const char *argstr, void *data) @@ -1760,12 +1782,13 @@ static void command_open(struct list *args, const char *argstr, void *data) continue; } - wordexp_t word; - if (wordexp(args->items[i], &word, 0) == 0) { - for (size_t j = 0; j < word.we_wordc; ++j) { - imv_navigator_add(imv->navigator, word.we_wordv[j], recursive); + glob_t g; + if (glob(args->items[i], GLOB_BRACE|GLOB_TILDE, NULL, &g) == 0) { + char **matches; + for (matches = g.gl_pathv; *matches; ++matches) { + imv_navigator_add(imv->navigator, *matches, recursive); } - wordfree(&word); + globfree(&g); } } } @@ -1995,22 +2018,61 @@ static void update_env_vars(struct imv *imv) static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, const char *format) { + pid_t pid; + int status, p[2]; /* read end, write end */ + + assert(buf_len > 0); + update_env_vars(imv); - size_t len = 0; - wordexp_t word; - setenv("IFS", "", 1); - if (wordexp(format, &word, 0) == 0) { - for (size_t i = 0; i < word.we_wordc; ++i) { - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]); - } - wordfree(&word); - } else { - len += snprintf(buf, buf_len, "error expanding text"); + if (pipe(p) == -1) + goto err; + + switch (pid = fork()) { + case -1: + close(p[0]); + close(p[1]); + goto err; + case 0: /* child */ + close(p[0]); + if (dup2(p[1], 1) != -1) { + execl("/bin/sh", "sh", "-c", "eval \"printf '%s ' $1\"", "sh", + format, (char *)NULL); + } + _exit(1); + default: /* parent */ + close(p[1]); + + size_t r, siz = buf_len - 1, tot = 0; + do { + r = read(p[0], buf, siz); + if (r == -1) { + goto err; + } else if (r == 0) { + break; + } + tot += r; + buf += r; + siz -= r; + } while (siz > 0); + + buf[tot] = '\0'; + while (tot > 0 && isspace((unsigned char)buf[tot-1])) + buf[--tot] = '\0'; + + close(p[0]); + + pid_t w; + do { + w = waitpid(pid, &status, 0); + } while (w == -1 && errno == EINTR); + + return tot > 0 ? tot-1 : tot; /* don't count the NUL terminator */ } - unsetenv("IFS"); - return len; +err: + snprintf(buf, buf_len, "error expanding text: %s", strerror(errno)); + return strlen(buf); } static size_t read_from_stdin(void **buffer)diff --git a/meson.build b/meson.build index f5568de..4c413f3 100644 --- a/meson.build +++ b/meson.build @@ -18,6 +18,7 @@ endif add_project_arguments('-DIMV_VERSION="@0@"'.format(version), language: 'c') add_project_arguments('-D_XOPEN_SOURCE=700', language: 'c') +add_project_arguments('-D_BSD_SOURCE', language: 'c') add_project_arguments('-DRSVG_DISABLE_DEPRECATION_WARNINGS', language: 'c') diff --git a/src/imv.c b/src/imv.c index 8380a12..7be8210 100644 --- a/src/imv.c +++ b/src/imv.c @@ -1,9 +1,12 @@ #include "imv.h" +#include <sys/wait.h> + #include <assert.h> #include <ctype.h> #include <errno.h> #include <getopt.h> +#include <glob.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -11,7 +14,6 @@ #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <wordexp.h> #include "backend.h" #include "binds.h" @@ -1417,40 +1419,6 @@ static void render_window(struct imv *imv) imv->cache_invalidated = false; } -static char *get_config_path(void) -{ - const char *config_paths[] = { - "$imv_config", - "$XDG_CONFIG_HOME/imv/config", - "$HOME/.config/imv/config", - "$HOME/.imv_config", - "$HOME/.imv/config", - "/usr/local/etc/imv_config", - "/etc/imv_config", - }; - - for (size_t i = 0; i < sizeof(config_paths) / sizeof(char*); ++i) { - wordexp_t word; - if (wordexp(config_paths[i], &word, 0) == 0) { - if (!word.we_wordv[0]) { - wordfree(&word); - continue; - } - - char *path = strdup(word.we_wordv[0]); - wordfree(&word); - - if (!path || access(path, R_OK) == -1) { - free(path); - continue; - } - - return path; - } - } - return NULL; -} - static bool parse_bool(const char *str) { return ( @@ -1611,26 +1579,80 @@ static int handle_ini_value(void *user, const char *section, const char *name, return 0; } +static int load_config(const char *path, struct imv *imv) +{ + int err; + + err = ini_parse(path, handle_ini_value, imv); + if (err > 0) + imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); + return err; +} + bool imv_load_config(struct imv *imv) { - char *path = get_config_path(); - if (!path) { - /* no config, no problem - we have defaults */ - return true; + char p[PATH_MAX]; + char *t; + int err; + + if ((t = getenv("imv_config")) != NULL) { + err = load_config(t, imv); + if (err > 0) + return false; + if (err == 0) + return true; } - bool result = true; + if ((t = getenv("XDG_CONFIG_HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + } - const int err = ini_parse(path, handle_ini_value, imv); - if (err == -1) { - imv_log(IMV_ERROR, "Unable to open config file: %s\n", path); - result = false; - } else if (err > 0) { - imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); - result = false; + if ((t = getenv("HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/.config/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv_config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } } - free(path); - return result; + + err = load_config("/usr/local/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + err = load_config("/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + /* no config, no problem - we have defaults */ + return true; } static void command_quit(struct list *args, const char *argstr, void *data) @@ -1760,12 +1782,13 @@ static void command_open(struct list *args, const char *argstr, void *data) continue; } - wordexp_t word; - if (wordexp(args->items[i], &word, 0) == 0) { - for (size_t j = 0; j < word.we_wordc; ++j) { - imv_navigator_add(imv->navigator, word.we_wordv[j], recursive); + glob_t g; + if (glob(args->items[i], GLOB_BRACE|GLOB_TILDE, NULL, &g) == 0) { + char **matches; + for (matches = g.gl_pathv; *matches; ++matches) { + imv_navigator_add(imv->navigator, *matches, recursive); } - wordfree(&word); + globfree(&g); } } } @@ -1995,22 +2018,61 @@ static void update_env_vars(struct imv *imv) static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, const char *format) { + pid_t pid; + int status, p[2]; /* read end, write end */ + + assert(buf_len > 0); + update_env_vars(imv); - size_t len = 0; - wordexp_t word; - setenv("IFS", "", 1); - if (wordexp(format, &word, 0) == 0) { - for (size_t i = 0; i < word.we_wordc; ++i) { - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]); - } - wordfree(&word); - } else { - len += snprintf(buf, buf_len, "error expanding text"); + if (pipe(p) == -1) + goto err; + + switch (pid = fork()) { + case -1: + close(p[0]); + close(p[1]); + goto err; + case 0: /* child */ + close(p[0]); + if (dup2(p[1], 1) != -1) { + execl("/bin/sh", "sh", "-c", "eval \"printf '%s ' $1\"", "sh", + format, (char *)NULL); + } + _exit(1); + default: /* parent */ + close(p[1]); + + size_t r, siz = buf_len - 1, tot = 0; + do { + r = read(p[0], buf, siz); + if (r == -1) { + goto err; + } else if (r == 0) { + break; + } + tot += r; + buf += r; + siz -= r; + } while (siz > 0); + + buf[tot] = '\0'; + while (tot > 0 && isspace((unsigned char)buf[tot-1])) + buf[--tot] = '\0'; + + close(p[0]); + + pid_t w; + do { + w = waitpid(pid, &status, 0); + } while (w != -1 && errno == EINTR); + + return tot > 0 ? tot-1 : tot; /* don't count the NUL terminator */ } - unsetenv("IFS"); - return len; +err: + snprintf(buf, buf_len, "error expanding text: %s", strerror(errno)); + return strlen(buf); } static size_t read_from_stdin(void **buffer)
Thanks, Omar Polo P.S.: please Cc me since I'm not subscribed to the list diff --git a/src/imv.c b/src/imv.c index 8380a12..113d59e 100644 --- a/src/imv.c +++ b/src/imv.c @@ -4,6 +4,7 @@ #include <ctype.h> #include <errno.h> #include <getopt.h> +#include <glob.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -11,7 +12,6 @@ #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <wordexp.h> #include "backend.h" #include "binds.h" @@ -1417,40 +1417,6 @@ static void render_window(struct imv *imv) imv->cache_invalidated = false; } -static char *get_config_path(void) -{ - const char *config_paths[] = { - "$imv_config", - "$XDG_CONFIG_HOME/imv/config", - "$HOME/.config/imv/config", - "$HOME/.imv_config", - "$HOME/.imv/config", - "/usr/local/etc/imv_config", - "/etc/imv_config", - }; - - for (size_t i = 0; i < sizeof(config_paths) / sizeof(char*); ++i) { - wordexp_t word; - if (wordexp(config_paths[i], &word, 0) == 0) { - if (!word.we_wordv[0]) { - wordfree(&word); - continue; - } - - char *path = strdup(word.we_wordv[0]); - wordfree(&word); - - if (!path || access(path, R_OK) == -1) { - free(path); - continue; - } - - return path; - } - } - return NULL; -} - static bool parse_bool(const char *str) { return ( @@ -1611,26 +1577,80 @@ static int handle_ini_value(void *user, const char *section, const char *name, return 0; } +static int load_config(const char *path, struct imv *imv) +{ + int err; + + err = ini_parse(path, handle_ini_value, imv); + if (err > 0) + imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); + return err; +} + bool imv_load_config(struct imv *imv) { - char *path = get_config_path(); - if (!path) { - /* no config, no problem - we have defaults */ - return true; + char p[PATH_MAX]; + char *t; + int err; + + if ((t = getenv("imv_config")) != NULL) { + err = load_config(t, imv); + if (err > 0) + return false; + if (err == 0) + return true; } - bool result = true; + if ((t = getenv("XDG_CONFIG_HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + } - const int err = ini_parse(path, handle_ini_value, imv); - if (err == -1) { - imv_log(IMV_ERROR, "Unable to open config file: %s\n", path); - result = false; - } else if (err > 0) { - imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); - result = false; + if ((t = getenv("HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/.config/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv_config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } } - free(path); - return result; + + err = load_config("/usr/local/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + err = load_config("/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + /* no config, no problem - we have defaults */ + return true; } static void command_quit(struct list *args, const char *argstr, void *data) @@ -1760,12 +1780,13 @@ static void command_open(struct list *args, const char *argstr, void *data) continue; } - wordexp_t word; - if (wordexp(args->items[i], &word, 0) == 0) { - for (size_t j = 0; j < word.we_wordc; ++j) { - imv_navigator_add(imv->navigator, word.we_wordv[j], recursive); + glob_t g; + if (glob(args->items[i], GLOB_BRACE|GLOB_TILDE, NULL, &g) == 0) { + char **matches; + for (matches = g.gl_pathv; *matches; ++matches) { + imv_navigator_add(imv->navigator, *matches, recursive); } - wordfree(&word); + globfree(&g); } } } @@ -1995,22 +2016,56 @@ static void update_env_vars(struct imv *imv) static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, const char *format) { + pid_t pid; + int p[2]; /* read end, write end */ + + assert(buf_len > 0); + update_env_vars(imv); - size_t len = 0; - wordexp_t word; - setenv("IFS", "", 1); - if (wordexp(format, &word, 0) == 0) { - for (size_t i = 0; i < word.we_wordc; ++i) { - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]); - } - wordfree(&word); - } else { - len += snprintf(buf, buf_len, "error expanding text"); + if (pipe(p) == -1) + goto err; + + switch (pid = fork()) { + case -1: + close(p[0]); + close(p[1]); + goto err; + case 0: /* child */ + close(p[0]); + setenv("IFS", "", 1); + if (dup2(p[1], 1) != -1) { + execl("/bin/sh", "sh", "-c", "eval \"printf '%s ' $1\"", "sh", + format, (char *)NULL); + } + _exit(1); + default: /* parent */ + close(p[1]); + + size_t r, siz = buf_len - 1, tot = 0; + do { + r = read(p[0], buf, siz); + if (r == -1) { + goto err; + } else if (r == 0) { + break; + } + tot += r; + buf += r; + siz -= r; + } while (siz > 0); + + buf[tot] = '\0'; + while (tot > 0 && isspace((unsigned char)buf[tot-1])) + buf[--tot] = '\0'; + + close(p[0]); + return tot > 0 ? tot-1 : tot; /* don't count the NUL terminator */ } - unsetenv("IFS"); - return len; +err: + snprintf(buf, buf_len, "error expanding text: %s", strerror(errno)); + return strlen(buf); } static size_t read_from_stdin(void **buffer)
Omar Polo <op@omarpolo.com> wrote: > woop, I've left by accident the unused variable `len'. > > GLOB_BRACE and GLOB_TILDE fails on other systems due to _XOPEN_SOURCE > being defined. For FreeBSD defining _BSD_SOURCE too should do it (it's > required on OpenBSD too), for glibc I have no idea. _GNU_SOURCE maybe? > > (or maybe drop _XOPEN_SOURCE.) > > Here's an updated diff without the unused variable. The previous diff lacked a wait(2) call to reap the child process spawned by generate_env_text. I'm also adding -D_BSD_SOURCE to get the GLOB_* defines. The reason I hit the limits mentioned in the first mail was due to the lack of the wait(2) call; now imv seems to work perfectly fine :)
Omar Polo <op@omarpolo.com>...aaand i can't type it seems. There was a typo in the waitpid loop, it should have been (w == -1) and not != -1. sorry for the noise. diff --git a/meson.build b/meson.build index f5568de..4c413f3 100644 --- a/meson.build +++ b/meson.build @@ -18,6 +18,7 @@ endif add_project_arguments('-DIMV_VERSION="@0@"'.format(version), language: 'c') add_project_arguments('-D_XOPEN_SOURCE=700', language: 'c') +add_project_arguments('-D_BSD_SOURCE', language: 'c') add_project_arguments('-DRSVG_DISABLE_DEPRECATION_WARNINGS', language: 'c') diff --git a/src/imv.c b/src/imv.c index 8380a12..6db9cc8 100644 --- a/src/imv.c +++ b/src/imv.c @@ -1,9 +1,12 @@ #include "imv.h" +#include <sys/wait.h> + #include <assert.h> #include <ctype.h> #include <errno.h> #include <getopt.h> +#include <glob.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -11,7 +14,6 @@ #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <wordexp.h> #include "backend.h" #include "binds.h" @@ -1417,40 +1419,6 @@ static void render_window(struct imv *imv) imv->cache_invalidated = false; } -static char *get_config_path(void) -{ - const char *config_paths[] = { - "$imv_config", - "$XDG_CONFIG_HOME/imv/config", - "$HOME/.config/imv/config", - "$HOME/.imv_config", - "$HOME/.imv/config", - "/usr/local/etc/imv_config", - "/etc/imv_config", - }; - - for (size_t i = 0; i < sizeof(config_paths) / sizeof(char*); ++i) { - wordexp_t word; - if (wordexp(config_paths[i], &word, 0) == 0) { - if (!word.we_wordv[0]) { - wordfree(&word); - continue; - } - - char *path = strdup(word.we_wordv[0]); - wordfree(&word); - - if (!path || access(path, R_OK) == -1) { - free(path); - continue; - } - - return path; - } - } - return NULL; -} - static bool parse_bool(const char *str) { return ( @@ -1611,26 +1579,80 @@ static int handle_ini_value(void *user, const char *section, const char *name, return 0; } +static int load_config(const char *path, struct imv *imv) +{ + int err; + + err = ini_parse(path, handle_ini_value, imv); + if (err > 0) + imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); + return err; +} + bool imv_load_config(struct imv *imv) { - char *path = get_config_path(); - if (!path) { - /* no config, no problem - we have defaults */ - return true; + char p[PATH_MAX]; + char *t; + int err; + + if ((t = getenv("imv_config")) != NULL) { + err = load_config(t, imv); + if (err > 0) + return false; + if (err == 0) + return true; } - bool result = true; + if ((t = getenv("XDG_CONFIG_HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + } - const int err = ini_parse(path, handle_ini_value, imv); - if (err == -1) { - imv_log(IMV_ERROR, "Unable to open config file: %s\n", path); - result = false; - } else if (err > 0) { - imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); - result = false; + if ((t = getenv("HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/.config/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv_config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } } - free(path); - return result; + + err = load_config("/usr/local/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + err = load_config("/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + /* no config, no problem - we have defaults */ + return true; } static void command_quit(struct list *args, const char *argstr, void *data) @@ -1760,12 +1782,13 @@ static void command_open(struct list *args, const char *argstr, void *data) continue; } - wordexp_t word; - if (wordexp(args->items[i], &word, 0) == 0) { - for (size_t j = 0; j < word.we_wordc; ++j) { - imv_navigator_add(imv->navigator, word.we_wordv[j], recursive); + glob_t g; + if (glob(args->items[i], GLOB_BRACE|GLOB_TILDE, NULL, &g) == 0) { + char **matches; + for (matches = g.gl_pathv; *matches; ++matches) { + imv_navigator_add(imv->navigator, *matches, recursive); } - wordfree(&word); + globfree(&g); } } } @@ -1995,22 +2018,61 @@ static void update_env_vars(struct imv *imv) static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, const char *format) { + pid_t pid; + int status, p[2]; /* read end, write end */ + + assert(buf_len > 0); + update_env_vars(imv); - size_t len = 0; - wordexp_t word; - setenv("IFS", "", 1); - if (wordexp(format, &word, 0) == 0) { - for (size_t i = 0; i < word.we_wordc; ++i) { - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]); - } - wordfree(&word); - } else { - len += snprintf(buf, buf_len, "error expanding text"); + if (pipe(p) == -1) + goto err; + + switch (pid = fork()) { + case -1: + close(p[0]); + close(p[1]); + goto err; + case 0: /* child */ + close(p[0]); + if (dup2(p[1], 1) != -1) { + execl("/bin/sh", "sh", "-c", "eval \"printf '%s ' $1\"", "sh", + format, (char *)NULL); + } + _exit(1); + default: /* parent */ + close(p[1]); + + size_t r, siz = buf_len - 1, tot = 0; + do { + r = read(p[0], buf, siz); + if (r == -1) { + goto err; + } else if (r == 0) { + break; + } + tot += r; + buf += r; + siz -= r; + } while (siz > 0); + + buf[tot] = '\0'; + while (tot > 0 && isspace((unsigned char)buf[tot-1])) + buf[--tot] = '\0'; + + close(p[0]); + + pid_t w; + do { + w = waitpid(pid, &status, 0); + } while (w == -1 && errno == EINTR); + + return tot > 0 ? tot-1 : tot; /* don't count the NUL terminator */ } - unsetenv("IFS"); - return len; +err: + snprintf(buf, buf_len, "error expanding text: %s", strerror(errno)); + return strlen(buf); } static size_t read_from_stdin(void **buffer)
diff --git a/meson.build b/meson.build index f5568de..4c413f3 100644 --- a/meson.build +++ b/meson.build @@ -18,6 +18,7 @@ endif add_project_arguments('-DIMV_VERSION="@0@"'.format(version), language: 'c') add_project_arguments('-D_XOPEN_SOURCE=700', language: 'c') +add_project_arguments('-D_BSD_SOURCE', language: 'c') add_project_arguments('-DRSVG_DISABLE_DEPRECATION_WARNINGS', language: 'c') diff --git a/src/imv.c b/src/imv.c index 8380a12..7be8210 100644 --- a/src/imv.c +++ b/src/imv.c @@ -1,9 +1,12 @@ #include "imv.h" +#include <sys/wait.h> + #include <assert.h> #include <ctype.h> #include <errno.h> #include <getopt.h> +#include <glob.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -11,7 +14,6 @@ #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <wordexp.h> #include "backend.h" #include "binds.h" @@ -1417,40 +1419,6 @@ static void render_window(struct imv *imv) imv->cache_invalidated = false; } -static char *get_config_path(void) -{ - const char *config_paths[] = { - "$imv_config", - "$XDG_CONFIG_HOME/imv/config", - "$HOME/.config/imv/config", - "$HOME/.imv_config", - "$HOME/.imv/config", - "/usr/local/etc/imv_config", - "/etc/imv_config", - }; - - for (size_t i = 0; i < sizeof(config_paths) / sizeof(char*); ++i) { - wordexp_t word; - if (wordexp(config_paths[i], &word, 0) == 0) { - if (!word.we_wordv[0]) { - wordfree(&word); - continue; - } - - char *path = strdup(word.we_wordv[0]); - wordfree(&word); - - if (!path || access(path, R_OK) == -1) { - free(path); - continue; - } - - return path; - } - } - return NULL; -} - static bool parse_bool(const char *str) { return ( @@ -1611,26 +1579,80 @@ static int handle_ini_value(void *user, const char *section, const char *name, return 0; } +static int load_config(const char *path, struct imv *imv) +{ + int err; + + err = ini_parse(path, handle_ini_value, imv); + if (err > 0) + imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); + return err; +} + bool imv_load_config(struct imv *imv) { - char *path = get_config_path(); - if (!path) { - /* no config, no problem - we have defaults */ - return true; + char p[PATH_MAX]; + char *t; + int err; + + if ((t = getenv("imv_config")) != NULL) { + err = load_config(t, imv); + if (err > 0) + return false; + if (err == 0) + return true; } - bool result = true; + if ((t = getenv("XDG_CONFIG_HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + } - const int err = ini_parse(path, handle_ini_value, imv); - if (err == -1) { - imv_log(IMV_ERROR, "Unable to open config file: %s\n", path); - result = false; - } else if (err > 0) { - imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); - result = false; + if ((t = getenv("HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/.config/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv_config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } } - free(path); - return result; + + err = load_config("/usr/local/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + err = load_config("/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + /* no config, no problem - we have defaults */ + return true; } static void command_quit(struct list *args, const char *argstr, void *data) @@ -1760,12 +1782,13 @@ static void command_open(struct list *args, const char *argstr, void *data) continue; } - wordexp_t word; - if (wordexp(args->items[i], &word, 0) == 0) { - for (size_t j = 0; j < word.we_wordc; ++j) { - imv_navigator_add(imv->navigator, word.we_wordv[j], recursive); + glob_t g; + if (glob(args->items[i], GLOB_BRACE|GLOB_TILDE, NULL, &g) == 0) { + char **matches; + for (matches = g.gl_pathv; *matches; ++matches) { + imv_navigator_add(imv->navigator, *matches, recursive); } - wordfree(&word); + globfree(&g); } } } @@ -1995,22 +2018,61 @@ static void update_env_vars(struct imv *imv) static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, const char *format) { + pid_t pid; + int status, p[2]; /* read end, write end */ + + assert(buf_len > 0); + update_env_vars(imv); - size_t len = 0; - wordexp_t word; - setenv("IFS", "", 1); - if (wordexp(format, &word, 0) == 0) { - for (size_t i = 0; i < word.we_wordc; ++i) { - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]); - } - wordfree(&word); - } else { - len += snprintf(buf, buf_len, "error expanding text"); + if (pipe(p) == -1) + goto err; + + switch (pid = fork()) { + case -1: + close(p[0]); + close(p[1]); + goto err; + case 0: /* child */ + close(p[0]); + if (dup2(p[1], 1) != -1) { + execl("/bin/sh", "sh", "-c", "eval \"printf '%s ' $1\"", "sh", + format, (char *)NULL); + } + _exit(1); + default: /* parent */ + close(p[1]); + + size_t r, siz = buf_len - 1, tot = 0; + do { + r = read(p[0], buf, siz); + if (r == -1) { + goto err; + } else if (r == 0) { + break; + } + tot += r; + buf += r; + siz -= r; + } while (siz > 0); + + buf[tot] = '\0'; + while (tot > 0 && isspace((unsigned char)buf[tot-1])) + buf[--tot] = '\0'; + + close(p[0]); + + pid_t w; + do { + w = waitpid(pid, &status, 0); + } while (w != -1 && errno == EINTR); + + return tot > 0 ? tot-1 : tot; /* don't count the NUL terminator */ } - unsetenv("IFS"); - return len; +err: + snprintf(buf, buf_len, "error expanding text: %s", strerror(errno)); + return strlen(buf); } static size_t read_from_stdin(void **buffer)
Omar Polo <op@omarpolo.com> wrote: > Omar Polo <op@omarpolo.com> wrote: > > woop, I've left by accident the unused variable `len'. > > > > GLOB_BRACE and GLOB_TILDE fails on other systems due to _XOPEN_SOURCE > > being defined. For FreeBSD defining _BSD_SOURCE too should do it (it's > > required on OpenBSD too), for glibc I have no idea. _GNU_SOURCE maybe? > > > > (or maybe drop _XOPEN_SOURCE.) > > > > Here's an updated diff without the unused variable. > > The previous diff lacked a wait(2) call to reap the child process > spawned by generate_env_text. I'm also adding -D_BSD_SOURCE to get > the GLOB_* defines. > > The reason I hit the limits mentioned in the first mail was due to the > lack of the wait(2) call; now imv seems to work perfectly fine :) ...aaand i can't type it seems. There was a typo in the waitpid loop, it should have been (w == -1) and not != -1. sorry for the noise. diff --git a/meson.build b/meson.build index f5568de..4c413f3 100644 --- a/meson.build +++ b/meson.build @@ -18,6 +18,7 @@ endif add_project_arguments('-DIMV_VERSION="@0@"'.format(version), language: 'c') add_project_arguments('-D_XOPEN_SOURCE=700', language: 'c') +add_project_arguments('-D_BSD_SOURCE', language: 'c') add_project_arguments('-DRSVG_DISABLE_DEPRECATION_WARNINGS', language: 'c') diff --git a/src/imv.c b/src/imv.c index 8380a12..6db9cc8 100644 --- a/src/imv.c +++ b/src/imv.c @@ -1,9 +1,12 @@ #include "imv.h" +#include <sys/wait.h> + #include <assert.h> #include <ctype.h> #include <errno.h> #include <getopt.h> +#include <glob.h> #include <limits.h> #include <pthread.h> #include <stdbool.h> @@ -11,7 +14,6 @@ #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <wordexp.h> #include "backend.h" #include "binds.h" @@ -1417,40 +1419,6 @@ static void render_window(struct imv *imv) imv->cache_invalidated = false; } -static char *get_config_path(void) -{ - const char *config_paths[] = { - "$imv_config", - "$XDG_CONFIG_HOME/imv/config", - "$HOME/.config/imv/config", - "$HOME/.imv_config", - "$HOME/.imv/config", - "/usr/local/etc/imv_config", - "/etc/imv_config", - }; - - for (size_t i = 0; i < sizeof(config_paths) / sizeof(char*); ++i) { - wordexp_t word; - if (wordexp(config_paths[i], &word, 0) == 0) { - if (!word.we_wordv[0]) { - wordfree(&word); - continue; - } - - char *path = strdup(word.we_wordv[0]); - wordfree(&word); - - if (!path || access(path, R_OK) == -1) { - free(path); - continue; - } - - return path; - } - } - return NULL; -} - static bool parse_bool(const char *str) { return ( @@ -1611,26 +1579,80 @@ static int handle_ini_value(void *user, const char *section, const char *name, return 0; } +static int load_config(const char *path, struct imv *imv) +{ + int err; + + err = ini_parse(path, handle_ini_value, imv); + if (err > 0) + imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); + return err; +} + bool imv_load_config(struct imv *imv) { - char *path = get_config_path(); - if (!path) { - /* no config, no problem - we have defaults */ - return true; + char p[PATH_MAX]; + char *t; + int err; + + if ((t = getenv("imv_config")) != NULL) { + err = load_config(t, imv); + if (err > 0) + return false; + if (err == 0) + return true; } - bool result = true; + if ((t = getenv("XDG_CONFIG_HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + } - const int err = ini_parse(path, handle_ini_value, imv); - if (err == -1) { - imv_log(IMV_ERROR, "Unable to open config file: %s\n", path); - result = false; - } else if (err > 0) { - imv_log(IMV_ERROR, "Error in config file: %s:%d\n", path, err); - result = false; + if ((t = getenv("HOME")) != NULL) { + if (snprintf(p, sizeof(p), "%s/.config/imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv_config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } + + if (snprintf(p, sizeof(p), "%s/.imv/config", t) < sizeof(p)) { + err = load_config(p, imv); + if (err > 0) + return false; + if (err == 0) + return true; + } } - free(path); - return result; + + err = load_config("/usr/local/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + err = load_config("/etc/imv_config", imv); + if (err > 0) + return false; + if (err == 0) + return true; + + /* no config, no problem - we have defaults */ + return true; } static void command_quit(struct list *args, const char *argstr, void *data) @@ -1760,12 +1782,13 @@ static void command_open(struct list *args, const char *argstr, void *data) continue; } - wordexp_t word; - if (wordexp(args->items[i], &word, 0) == 0) { - for (size_t j = 0; j < word.we_wordc; ++j) { - imv_navigator_add(imv->navigator, word.we_wordv[j], recursive); + glob_t g; + if (glob(args->items[i], GLOB_BRACE|GLOB_TILDE, NULL, &g) == 0) { + char **matches; + for (matches = g.gl_pathv; *matches; ++matches) { + imv_navigator_add(imv->navigator, *matches, recursive); } - wordfree(&word); + globfree(&g); } } } @@ -1995,22 +2018,61 @@ static void update_env_vars(struct imv *imv) static size_t generate_env_text(struct imv *imv, char *buf, size_t buf_len, const char *format) { + pid_t pid; + int status, p[2]; /* read end, write end */ + + assert(buf_len > 0); + update_env_vars(imv); - size_t len = 0; - wordexp_t word; - setenv("IFS", "", 1); - if (wordexp(format, &word, 0) == 0) { - for (size_t i = 0; i < word.we_wordc; ++i) { - len += snprintf(buf + len, buf_len - len, "%s ", word.we_wordv[i]); - } - wordfree(&word); - } else { - len += snprintf(buf, buf_len, "error expanding text"); + if (pipe(p) == -1) + goto err; + + switch (pid = fork()) { + case -1: + close(p[0]); + close(p[1]); + goto err; + case 0: /* child */ + close(p[0]); + if (dup2(p[1], 1) != -1) { + execl("/bin/sh", "sh", "-c", "eval \"printf '%s ' $1\"", "sh", + format, (char *)NULL); + } + _exit(1); + default: /* parent */ + close(p[1]); + + size_t r, siz = buf_len - 1, tot = 0; + do { + r = read(p[0], buf, siz); + if (r == -1) { + goto err; + } else if (r == 0) { + break; + } + tot += r; + buf += r; + siz -= r; + } while (siz > 0); + + buf[tot] = '\0'; + while (tot > 0 && isspace((unsigned char)buf[tot-1])) + buf[--tot] = '\0'; + + close(p[0]); + + pid_t w; + do { + w = waitpid(pid, &status, 0); + } while (w == -1 && errno == EINTR); + + return tot > 0 ? tot-1 : tot; /* don't count the NUL terminator */ } - unsetenv("IFS"); - return len; +err: + snprintf(buf, buf_len, "error expanding text: %s", strerror(errno)); + return strlen(buf); } static size_t read_from_stdin(void **buffer)