The read() underlying fgets() can be interrupted by a signal handler causing fgets() to return NULL. This was less of an issue in the past because signals generally resulted in the termination of the program, the recently added SIGWINCH handler does not. Replace all fgets calls with a wrapper that retries on EINTR. Signed-off-by: Andrew Gregory --- fgets_eintr is a horrible name, suggestions welcome... This is needed in addition to the previous patch for non-pacman front-ends not kind enough to set SA_RESTART. lib/libalpm/be_local.c | 14 +++++++------- lib/libalpm/trans.c | 2 +- lib/libalpm/util.c | 3 ++- src/common/util-common.c | 17 +++++++++++++++++ src/common/util-common.h | 3 +++ src/pacman/ini.c | 2 +- src/pacman/util.c | 8 ++++---- 7 files changed, 35 insertions(+), 14 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 9a9bdef..48bd0fd 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -573,7 +573,7 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, } #define READ_NEXT() do { \ - if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \ + if(fgets_eintr(line, sizeof(line), fp) == NULL && !feof(fp)) goto error; \ _alpm_strip_newline(line, 0); \ } while(0) @@ -584,7 +584,7 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, #define READ_AND_STORE_ALL(f) do { \ char *linedup; \ - if(fgets(line, sizeof(line), fp) == NULL) {\ + if(fgets_eintr(line, sizeof(line), fp) == NULL) {\ if(!feof(fp)) goto error; else break; \ } \ if(_alpm_strip_newline(line, 0) == 0) break; \ @@ -593,7 +593,7 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t *info, } while(1) /* note the while(1) and not (0) */ #define READ_AND_SPLITDEP(f) do { \ - if(fgets(line, sizeof(line), fp) == NULL) {\ + if(fgets_eintr(line, sizeof(line), fp) == NULL) {\ if(!feof(fp)) goto error; else break; \ } \ if(_alpm_strip_newline(line, 0) == 0) break; \ @@ -639,7 +639,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) } free(path); while(!feof(fp)) { - if(fgets(line, sizeof(line), fp) == NULL && !feof(fp)) { + if(fgets_eintr(line, sizeof(line), fp) == NULL && !feof(fp)) { goto error; } if(_alpm_strip_newline(line, 0) == 0) { @@ -728,13 +728,13 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) goto error; } free(path); - while(fgets(line, sizeof(line), fp)) { + while(fgets_eintr(line, sizeof(line), fp)) { _alpm_strip_newline(line, 0); if(strcmp(line, "%FILES%") == 0) { size_t files_count = 0, files_size = 0, len; alpm_file_t *files = NULL; - while(fgets(line, sizeof(line), fp) && + while(fgets_eintr(line, sizeof(line), fp) && (len = _alpm_strip_newline(line, 0))) { if(!_alpm_greedy_grow((void **)&files, &files_size, (files_size ? files_size + sizeof(alpm_file_t) : 8 * sizeof(alpm_file_t)))) { @@ -754,7 +754,7 @@ static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq) info->files.count = files_count; info->files.files = files; } else if(strcmp(line, "%BACKUP%") == 0) { - while(fgets(line, sizeof(line), fp) && _alpm_strip_newline(line, 0)) { + while(fgets_eintr(line, sizeof(line), fp) && _alpm_strip_newline(line, 0)) { alpm_backup_t *backup; CALLOC(backup, 1, sizeof(alpm_backup_t), goto error); if(_alpm_split_backup(line, &backup)) { diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index e5328c5..6cb7eff 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -275,7 +275,7 @@ static int grep(const char *fn, const char *needle) } while(!feof(fp)) { char line[1024]; - if(fgets(line, sizeof(line), fp) == NULL) { + if(fgets_eintr(line, sizeof(line), fp) == NULL) { continue; } /* TODO: this will not work if the search string diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 6dab0de..c58c5de 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -576,8 +576,9 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) .type = ALPM_EVENT_SCRIPTLET_INFO, .line = line }; - if(fgets(line, PATH_MAX, pipe_file) == NULL) + if(fgets_eintr(line, PATH_MAX, pipe_file) == NULL) { break; + } alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line); EVENT(handle, &event); } diff --git a/src/common/util-common.c b/src/common/util-common.c index 3316eae..96dc463 100644 --- a/src/common/util-common.c +++ b/src/common/util-common.c @@ -17,6 +17,7 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */ +#include #include #include @@ -102,6 +103,22 @@ int llstat(char *path, struct stat *buf) return ret; } +/** Wrapper around fgets() which properly handles EINTR + * @param s string to read into + * @param size maximum length to read + * @param stream stream to read from + * @return value returned by fgets() + */ +char *fgets_eintr(char *s, int size, FILE *stream) +{ + char *ret; + do { + errno = 0; + ret = fgets(s, size, stream); + } while(ret == NULL && !feof(stream) && errno == EINTR); + return ret; +} + #ifndef HAVE_STRNDUP /* A quick and dirty implementation derived from glibc */ /** Determines the length of a fixed-size string. diff --git a/src/common/util-common.h b/src/common/util-common.h index 576702f..2ad23ba 100644 --- a/src/common/util-common.h +++ b/src/common/util-common.h @@ -20,6 +20,7 @@ #ifndef _PM_UTIL_COMMON_H #define _PM_UTIL_COMMON_H +#include #include /* struct stat */ const char *mbasename(const char *path); @@ -27,6 +28,8 @@ char *mdirname(const char *path); int llstat(char *path, struct stat *buf); +char *fgets_eintr(char *s, int size, FILE *stream); + #ifndef HAVE_STRNDUP char *strndup(const char *s, size_t n); #endif diff --git a/src/pacman/ini.c b/src/pacman/ini.c index 2a3ef0e..df27272 100644 --- a/src/pacman/ini.c +++ b/src/pacman/ini.c @@ -66,7 +66,7 @@ static int _parse_ini(const char *file, ini_parser_fn cb, void *data, goto cleanup; } - while(fgets(line, PATH_MAX, fp)) { + while(fgets_eintr(line, PATH_MAX, fp)) { char *key, *value, *ptr; size_t line_len; diff --git a/src/pacman/util.c b/src/pacman/util.c index 6a095fd..f665b16 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1403,7 +1403,7 @@ int multiselect_question(char *array, int count) flush_term_input(fileno(stdin)); - if(fgets(response, response_len, stdin)) { + if(fgets_eintr(response, response_len, stdin)) { const size_t response_incr = 64; size_t len; /* handle buffer not being large enough to read full line case */ @@ -1416,7 +1416,7 @@ int multiselect_question(char *array, int count) lastchar = response + response_len - 1; /* sentinel byte */ *lastchar = 1; - if(fgets(response + response_len - response_incr - 1, + if(fgets_eintr(response + response_len - response_incr - 1, response_incr + 1, stdin) == 0) { free(response); return -1; @@ -1467,7 +1467,7 @@ int select_question(int count) flush_term_input(fileno(stdin)); - if(fgets(response, sizeof(response), stdin)) { + if(fgets_eintr(response, sizeof(response), stdin)) { size_t len = strtrim(response); if(len > 0) { int n; @@ -1521,7 +1521,7 @@ static int question(short preset, const char *format, va_list args) flush_term_input(fd_in); - if(fgets(response, sizeof(response), stdin)) { + if(fgets_eintr(response, sizeof(response), stdin)) { size_t len = strtrim(response); if(len == 0) { return preset; -- 2.0.1