diff options
-rw-r--r-- | changes/bug20988 | 4 | ||||
-rw-r--r-- | src/common/compat.c | 39 | ||||
-rw-r--r-- | src/common/compat.h | 2 | ||||
-rw-r--r-- | src/common/util.c | 4 | ||||
-rw-r--r-- | src/or/dirserv.c | 4 | ||||
-rw-r--r-- | src/or/geoip.c | 2 | ||||
-rw-r--r-- | src/test/test_util.c | 22 |
7 files changed, 63 insertions, 14 deletions
diff --git a/changes/bug20988 b/changes/bug20988 new file mode 100644 index 0000000000..b1d73f08e7 --- /dev/null +++ b/changes/bug20988 @@ -0,0 +1,4 @@ + o Minor bugfixes (portability): + - Add Tor compatibility function for fgets(3) due to inconsistency of + returned values in different supported C libraries. This fixes unit test + failures reported on FreeBSD. Fixes bug 20988. diff --git a/src/common/compat.c b/src/common/compat.c index 0dbede6bed..da4283fbaa 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -3476,6 +3476,45 @@ tor_getpass(const char *prompt, char *output, size_t buflen) #endif } +/** Read at most one less than the number of characters specified by + * <b>size</b> from the given <b>stream</b> and store it in <b>str</b>. + * + * Reading stops when a newline character is found or at EOF or error. If any + * characters are read and there's no error, a trailing NUL byte is appended to + * the end of <b>str</b>. + * + * Upon successful completion, this function returns a pointer to the string + * <b>str</b>. If EOF occurs before any characters are read the function will + * return NULL and the content of <b>str</b> is unchanged. Upon error, the + * function returns NULL and the caller must check for error using foef(3) and + * ferror(3). + */ +char * +tor_fgets(char *str, int size, FILE *stream) +{ + char *ret; + + /* Reset errno before our call to fgets(3) to avoid a situation where the + * caller is calling us again because we just returned NULL and errno == + * EAGAIN, but when they call us again we will always return NULL because the + * error flag on the file handler remains set and errno is set to EAGAIN. + */ + errno = 0; + + ret = fgets(str, size, stream); + + /* FreeBSD, OpenBSD, Linux (glibc), and Linux (musl) seem to disagree about + * what to do in the given situation. We check if the stream has been flagged + * with an error-bit and return NULL in that situation if errno is also set + * to EAGAIN. + */ + if (ferror(stream) && errno == EAGAIN) { + return NULL; + } + + return ret; +} + /** Return the amount of free disk space we have permission to use, in * bytes. Return -1 if the amount of free space can't be determined. */ int64_t diff --git a/src/common/compat.h b/src/common/compat.h index ee1c9454de..1f51ece61f 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -740,6 +740,8 @@ STATIC int tor_ersatz_socketpair(int family, int type, int protocol, ssize_t tor_getpass(const char *prompt, char *output, size_t buflen); +char *tor_fgets(char *str, int size, FILE *stream); + /* This needs some of the declarations above so we include it here. */ #include "compat_threads.h" diff --git a/src/common/util.c b/src/common/util.c index f980fa296c..a69d887e30 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -5020,7 +5020,7 @@ tor_read_all_handle(FILE *h, char *buf, size_t count, while (numread != count) { /* Use fgets because that is what we use in log_from_pipe() */ - retval = fgets(buf+numread, (int)(count-numread), h); + retval = tor_fgets(buf+numread, (int)(count-numread), h); if (NULL == retval) { if (feof(h)) { log_debug(LD_GENERAL, "fgets() reached end of file"); @@ -5355,7 +5355,7 @@ get_string_from_pipe(FILE *stream, char *buf_out, size_t count) tor_assert(count <= INT_MAX); - retval = fgets(buf_out, (int)count, stream); + retval = tor_fgets(buf_out, (int)count, stream); if (!retval) { if (feof(stream)) { diff --git a/src/or/dirserv.c b/src/or/dirserv.c index f01668adcb..2c3ce100df 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -2701,7 +2701,7 @@ dirserv_read_measured_bandwidths(const char *from_file, return -1; } - if (!fgets(line, sizeof(line), fp) + if (!tor_fgets(line, sizeof(line), fp) || !strlen(line) || line[strlen(line)-1] != '\n') { log_warn(LD_DIRSERV, "Long or truncated time in bandwidth file: %s", escaped(line)); @@ -2731,7 +2731,7 @@ dirserv_read_measured_bandwidths(const char *from_file, while (!feof(fp)) { measured_bw_line_t parsed_line; - if (fgets(line, sizeof(line), fp) && strlen(line)) { + if (tor_fgets(line, sizeof(line), fp) && strlen(line)) { if (measured_bw_line_parse(&parsed_line, line) != -1) { /* Also cache the line for dirserv_get_bandwidth_for_router() */ dirserv_cache_measured_bw(&parsed_line, file_time); diff --git a/src/or/geoip.c b/src/or/geoip.c index 74811ea643..a8dc807c19 100644 --- a/src/or/geoip.c +++ b/src/or/geoip.c @@ -346,7 +346,7 @@ geoip_load_file(sa_family_t family, const char *filename) (family == AF_INET) ? "IPv4" : "IPv6", filename); while (!feof(f)) { char buf[512]; - if (fgets(buf, (int)sizeof(buf), f) == NULL) + if (tor_fgets(buf, (int)sizeof(buf), f) == NULL) break; crypto_digest_add_bytes(geoip_digest_env, buf, strlen(buf)); /* FFFF track full country name. */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 5b4d5b7703..3e4d45d35b 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -3972,47 +3972,50 @@ test_util_fgets_eagain(void *ptr) /* Send in a partial line */ retlen = write(test_pipe[1], "A", 1); tt_int_op(retlen, OP_EQ, 1); - retptr = fgets(buf, sizeof(buf), test_stream); + retptr = tor_fgets(buf, sizeof(buf), test_stream); tt_int_op(errno, OP_EQ, EAGAIN); - tt_ptr_op(retptr, OP_EQ, buf); + tt_ptr_op(retptr, OP_EQ, NULL); tt_str_op(buf, OP_EQ, "A"); errno = 0; /* Send in the rest */ retlen = write(test_pipe[1], "B\n", 2); tt_int_op(retlen, OP_EQ, 2); - retptr = fgets(buf, sizeof(buf), test_stream); + retptr = tor_fgets(buf, sizeof(buf), test_stream); tt_int_op(errno, OP_EQ, 0); tt_ptr_op(retptr, OP_EQ, buf); tt_str_op(buf, OP_EQ, "B\n"); errno = 0; + memset(buf, '\0', sizeof(buf)); /* Send in a full line */ retlen = write(test_pipe[1], "CD\n", 3); tt_int_op(retlen, OP_EQ, 3); - retptr = fgets(buf, sizeof(buf), test_stream); + retptr = tor_fgets(buf, sizeof(buf), test_stream); tt_int_op(errno, OP_EQ, 0); tt_ptr_op(retptr, OP_EQ, buf); tt_str_op(buf, OP_EQ, "CD\n"); errno = 0; + memset(buf, '\0', sizeof(buf)); /* Send in a partial line */ retlen = write(test_pipe[1], "E", 1); tt_int_op(retlen, OP_EQ, 1); - retptr = fgets(buf, sizeof(buf), test_stream); + retptr = tor_fgets(buf, sizeof(buf), test_stream); tt_int_op(errno, OP_EQ, EAGAIN); - tt_ptr_op(retptr, OP_EQ, buf); + tt_ptr_op(retptr, OP_EQ, NULL); tt_str_op(buf, OP_EQ, "E"); errno = 0; /* Send in the rest */ retlen = write(test_pipe[1], "F\n", 2); tt_int_op(retlen, OP_EQ, 2); - retptr = fgets(buf, sizeof(buf), test_stream); + retptr = tor_fgets(buf, sizeof(buf), test_stream); tt_int_op(errno, OP_EQ, 0); tt_ptr_op(retptr, OP_EQ, buf); tt_str_op(buf, OP_EQ, "F\n"); errno = 0; + memset(buf, '\0', sizeof(buf)); /* Send in a full line and close */ retlen = write(test_pipe[1], "GH", 2); @@ -4020,14 +4023,14 @@ test_util_fgets_eagain(void *ptr) retval = close(test_pipe[1]); tt_int_op(retval, OP_EQ, 0); test_pipe[1] = -1; - retptr = fgets(buf, sizeof(buf), test_stream); + retptr = tor_fgets(buf, sizeof(buf), test_stream); tt_int_op(errno, OP_EQ, 0); tt_ptr_op(retptr, OP_EQ, buf); tt_str_op(buf, OP_EQ, "GH"); errno = 0; /* Check for EOF */ - retptr = fgets(buf, sizeof(buf), test_stream); + retptr = tor_fgets(buf, sizeof(buf), test_stream); tt_int_op(errno, OP_EQ, 0); tt_ptr_op(retptr, OP_EQ, NULL); retval = feof(test_stream); @@ -4036,6 +4039,7 @@ test_util_fgets_eagain(void *ptr) /* Check that buf is unchanged according to C99 and C11 */ tt_str_op(buf, OP_EQ, "GH"); + memset(buf, '\0', sizeof(buf)); done: if (test_stream != NULL) |