summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--changes/bug209884
-rw-r--r--src/common/compat.c39
-rw-r--r--src/common/compat.h2
-rw-r--r--src/common/util.c4
-rw-r--r--src/or/dirserv.c4
-rw-r--r--src/or/geoip.c2
-rw-r--r--src/test/test_util.c22
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)