[tor-commits] [tor/master] Remove fgets() compatbility function and related tests.
nickm at torproject.org
nickm at torproject.org
Thu Mar 9 13:38:51 UTC 2017
commit 02fc0a5ecfc917e9261596816e926468db976453
Author: Alexander Færøy <ahf at torproject.org>
Date: Wed Mar 8 23:11:42 2017 +0100
Remove fgets() compatbility function and related tests.
This patch removes the `tor_fgets()` wrapper around `fgets(3)` since it
is no longer needed. The function was created due to inconsistency
between the returned values of `fgets(3)` on different versions of Unix
when using `fgets(3)` on non-blocking file descriptors, but with the
recent changes in bug #21654 we switch from unbuffered to direct I/O on
non-blocking file descriptors in our utility module.
We continue to use `fgets(3)` directly in the geoip and dirserv module
since this usage is considered safe.
This patch also removes the test-case that was created to detect
differences in the implementation of `fgets(3)` as well as the changes
file since these changes was not included in any releases yet.
See: https://bugs.torproject.org/21654
---
changes/bug20988 | 4 --
src/common/compat.c | 39 ------------------
src/common/compat.h | 2 -
src/or/dirserv.c | 4 +-
src/or/geoip.c | 2 +-
src/test/test_util.c | 111 ---------------------------------------------------
6 files changed, 3 insertions(+), 159 deletions(-)
diff --git a/changes/bug20988 b/changes/bug20988
deleted file mode 100644
index b1d73f0..0000000
--- a/changes/bug20988
+++ /dev/null
@@ -1,4 +0,0 @@
- 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 da4283f..0dbede6 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -3476,45 +3476,6 @@ 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 1f51ece..ee1c945 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -740,8 +740,6 @@ 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/or/dirserv.c b/src/or/dirserv.c
index 2c3ce10..f01668a 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 (!tor_fgets(line, sizeof(line), fp)
+ if (!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 (tor_fgets(line, sizeof(line), fp) && strlen(line)) {
+ if (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 a8dc807..74811ea 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 (tor_fgets(buf, (int)sizeof(buf), f) == NULL)
+ if (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 640935a..130a019 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -3940,116 +3940,6 @@ test_util_exit_status(void *ptr)
#endif
#ifndef _WIN32
-/* Check that fgets with a non-blocking pipe returns partial lines and sets
- * EAGAIN, returns full lines and sets no error, and returns NULL on EOF and
- * sets no error */
-static void
-test_util_fgets_eagain(void *ptr)
-{
- int test_pipe[2] = {-1, -1};
- int retval;
- ssize_t retlen;
- char *retptr;
- FILE *test_stream = NULL;
- char buf[4] = { 0 };
-
- (void)ptr;
-
- errno = 0;
-
- /* Set up a pipe to test on */
- retval = pipe(test_pipe);
- tt_int_op(retval, OP_EQ, 0);
-
- /* Set up the read-end to be non-blocking */
- retval = fcntl(test_pipe[0], F_SETFL, O_NONBLOCK);
- tt_int_op(retval, OP_EQ, 0);
-
- /* Open it as a stdio stream */
- test_stream = fdopen(test_pipe[0], "r");
- tt_ptr_op(test_stream, OP_NE, NULL);
-
- /* Send in a partial line */
- retlen = write(test_pipe[1], "A", 1);
- tt_int_op(retlen, OP_EQ, 1);
- retptr = tor_fgets(buf, sizeof(buf), test_stream);
- tt_int_op(errno, OP_EQ, EAGAIN);
- 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 = 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 = 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 = tor_fgets(buf, sizeof(buf), test_stream);
- tt_int_op(errno, OP_EQ, EAGAIN);
- 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 = 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);
- tt_int_op(retlen, OP_EQ, 2);
- retval = close(test_pipe[1]);
- tt_int_op(retval, OP_EQ, 0);
- test_pipe[1] = -1;
- 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 = 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);
- tt_int_op(retval, OP_NE, 0);
- errno = 0;
-
- /* 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)
- fclose(test_stream);
- if (test_pipe[0] != -1)
- close(test_pipe[0]);
- if (test_pipe[1] != -1)
- close(test_pipe[1]);
-}
-
static void
test_util_string_from_pipe(void *ptr)
{
@@ -5839,7 +5729,6 @@ struct testcase_t util_tests[] = {
UTIL_TEST(num_cpus, 0),
UTIL_TEST_WIN_ONLY(load_win_lib, 0),
UTIL_TEST_NO_WIN(exit_status, 0),
- UTIL_TEST_NO_WIN(fgets_eagain, 0),
UTIL_TEST_NO_WIN(string_from_pipe, 0),
UTIL_TEST(format_hex_number, 0),
UTIL_TEST(format_dec_number, 0),
More information about the tor-commits
mailing list