commit 79f73ab330c0e643ba1ec2eb0633f0f80525a083 Author: Nick Mathewson nickm@torproject.org Date: Fri Jun 22 11:40:20 2018 -0400
Finally extract the log library and make it build.
This patch: - introduces an fdio module for low-level fd functions that don't need to log. - moves the responsibility for opening files outside of torlog.c, so it won't need to call tor_open_cloexec. --- Makefile.am | 2 + src/common/compat.c | 75 ------------------------------- src/common/compat.h | 5 --- src/common/util.c | 1 + src/include.am | 1 + src/lib/fdio/.may_include | 4 ++ src/lib/fdio/fdio.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++ src/lib/fdio/fdio.h | 17 ++++++++ src/lib/fdio/include.am | 17 ++++++++ src/lib/log/torlog.c | 54 ++++++++--------------- src/lib/log/torlog.h | 6 ++- src/or/config.c | 20 ++++++++- src/or/config.h | 4 +- src/or/keypin.c | 2 +- src/or/microdesc.c | 4 +- src/test/test_logging.c | 5 ++- src/test/test_util.c | 1 + 17 files changed, 204 insertions(+), 123 deletions(-)
diff --git a/Makefile.am b/Makefile.am index b6200c6a2..61451ed26 100644 --- a/Makefile.am +++ b/Makefile.am @@ -42,6 +42,7 @@ TOR_UTIL_LIBS = \ src/common/libor.a \ src/lib/libtor-log.a \ src/lib/libtor-lock.a \ + src/lib/libtor-fdio.a \ src/lib/libtor-container.a \ src/lib/libtor-string.a \ src/lib/libtor-malloc.a \ @@ -56,6 +57,7 @@ TOR_UTIL_TESTING_LIBS = \ src/common/libor-testing.a \ src/lib/libtor-log-testing.a \ src/lib/libtor-lock-testing.a \ + src/lib/libtor-fdio-testing.a \ src/lib/libtor-container-testing.a \ src/lib/libtor-string-testing.a \ src/lib/libtor-malloc-testing.a \ diff --git a/src/common/compat.c b/src/common/compat.c index 1dcd06581..467c51d6e 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -733,80 +733,6 @@ tor_lockfile_unlock(tor_lockfile_t *lockfile) tor_free(lockfile); }
-/** @{ */ -/** Some old versions of Unix didn't define constants for these values, - * and instead expect you to say 0, 1, or 2. */ -#ifndef SEEK_SET -#define SEEK_SET 0 -#endif -#ifndef SEEK_CUR -#define SEEK_CUR 1 -#endif -#ifndef SEEK_END -#define SEEK_END 2 -#endif -/** @} */ - -/** Return the position of <b>fd</b> with respect to the start of the file. */ -off_t -tor_fd_getpos(int fd) -{ -#ifdef _WIN32 - return (off_t) _lseek(fd, 0, SEEK_CUR); -#else - return (off_t) lseek(fd, 0, SEEK_CUR); -#endif -} - -/** Move <b>fd</b> to the end of the file. Return -1 on error, 0 on success. - * If the file is a pipe, do nothing and succeed. - **/ -int -tor_fd_seekend(int fd) -{ -#ifdef _WIN32 - return _lseek(fd, 0, SEEK_END) < 0 ? -1 : 0; -#else - off_t rc = lseek(fd, 0, SEEK_END) < 0 ? -1 : 0; -#ifdef ESPIPE - /* If we get an error and ESPIPE, then it's a pipe or a socket of a fifo: - * no need to worry. */ - if (rc < 0 && errno == ESPIPE) - rc = 0; -#endif /* defined(ESPIPE) */ - return (rc < 0) ? -1 : 0; -#endif /* defined(_WIN32) */ -} - -/** Move <b>fd</b> to position <b>pos</b> in the file. Return -1 on error, 0 - * on success. */ -int -tor_fd_setpos(int fd, off_t pos) -{ -#ifdef _WIN32 - return _lseek(fd, pos, SEEK_SET) < 0 ? -1 : 0; -#else - return lseek(fd, pos, SEEK_SET) < 0 ? -1 : 0; -#endif -} - -/** Replacement for ftruncate(fd, 0): move to the front of the file and remove - * all the rest of the file. Return -1 on error, 0 on success. */ -int -tor_ftruncate(int fd) -{ - /* Rumor has it that some versions of ftruncate do not move the file pointer. - */ - if (tor_fd_setpos(fd, 0) < 0) - return -1; - -#ifdef _WIN32 - return _chsize(fd, 0); -#else - return ftruncate(fd, 0); -#endif -} - #undef DEBUG_SOCKET_COUNTING #ifdef DEBUG_SOCKET_COUNTING /** A bitarray of all fds that should be passed to tor_socket_close(). Only @@ -2641,7 +2567,6 @@ compute_num_cpus(void) return num_cpus; }
- /** As localtime_r, but defined for platforms that don't have it: * * Convert *<b>timep</b> to a struct tm in local time, and store the value in diff --git a/src/common/compat.h b/src/common/compat.h index 605438cc6..f91c22425 100644 --- a/src/common/compat.h +++ b/src/common/compat.h @@ -160,11 +160,6 @@ tor_lockfile_t *tor_lockfile_lock(const char *filename, int blocking, int *locked_out); void tor_lockfile_unlock(tor_lockfile_t *lockfile);
-off_t tor_fd_getpos(int fd); -int tor_fd_setpos(int fd, off_t pos); -int tor_fd_seekend(int fd); -int tor_ftruncate(int fd); - int64_t tor_get_avail_disk_space(const char *path);
#ifdef _WIN32 diff --git a/src/common/util.c b/src/common/util.c index 986792ff2..fa95f933c 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -19,6 +19,7 @@ #include "lib/crypt_ops/crypto_digest.h" #include "lib/cc/torint.h" #include "lib/container/smartlist.h" +#include "lib/fdio/fdio.h" #include "common/address.h" #include "common/sandbox.h" #include "lib/err/backtrace.h" diff --git a/src/include.am b/src/include.am index 7eb6c069a..1d2a037e9 100644 --- a/src/include.am +++ b/src/include.am @@ -6,6 +6,7 @@ include src/lib/compress/include.am include src/lib/container/include.am include src/lib/crypt_ops/include.am include src/lib/defs/include.am +include src/lib/fdio/include.am include src/lib/include.libdonna.am include src/lib/intmath/include.am include src/lib/lock/include.am diff --git a/src/lib/fdio/.may_include b/src/lib/fdio/.may_include new file mode 100644 index 000000000..30fd277b8 --- /dev/null +++ b/src/lib/fdio/.may_include @@ -0,0 +1,4 @@ +orconfig.h +lib/cc/*.h +lib/err/*.h +lib/fdio/*.h diff --git a/src/lib/fdio/fdio.c b/src/lib/fdio/fdio.c new file mode 100644 index 000000000..b62207432 --- /dev/null +++ b/src/lib/fdio/fdio.c @@ -0,0 +1,109 @@ +/* Copyright (c) 2003-2004, Roger Dingledine + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#include "orconfig.h" + +#ifdef HAVE_UNISTD_H +#include <unistd.h> +#endif +#ifdef _WIN32 +#include <windows.h> +#endif + +#include "lib/fdio/fdio.h" +#include "lib/cc/torint.h" +#include "lib/err/torerr.h" + +#include <stdlib.h> + +/** @{ */ +/** Some old versions of Unix didn't define constants for these values, + * and instead expect you to say 0, 1, or 2. */ +#ifndef SEEK_SET +#define SEEK_SET 0 +#endif +#ifndef SEEK_CUR +#define SEEK_CUR 1 +#endif +#ifndef SEEK_END +#define SEEK_END 2 +#endif +/** @} */ + +/** Return the position of <b>fd</b> with respect to the start of the file. */ +off_t +tor_fd_getpos(int fd) +{ +#ifdef _WIN32 + return (off_t) _lseek(fd, 0, SEEK_CUR); +#else + return (off_t) lseek(fd, 0, SEEK_CUR); +#endif +} + +/** Move <b>fd</b> to the end of the file. Return -1 on error, 0 on success. + * If the file is a pipe, do nothing and succeed. + **/ +int +tor_fd_seekend(int fd) +{ +#ifdef _WIN32 + return _lseek(fd, 0, SEEK_END) < 0 ? -1 : 0; +#else + off_t rc = lseek(fd, 0, SEEK_END) < 0 ? -1 : 0; +#ifdef ESPIPE + /* If we get an error and ESPIPE, then it's a pipe or a socket of a fifo: + * no need to worry. */ + if (rc < 0 && errno == ESPIPE) + rc = 0; +#endif /* defined(ESPIPE) */ + return (rc < 0) ? -1 : 0; +#endif /* defined(_WIN32) */ +} + +/** Move <b>fd</b> to position <b>pos</b> in the file. Return -1 on error, 0 + * on success. */ +int +tor_fd_setpos(int fd, off_t pos) +{ +#ifdef _WIN32 + return _lseek(fd, pos, SEEK_SET) < 0 ? -1 : 0; +#else + return lseek(fd, pos, SEEK_SET) < 0 ? -1 : 0; +#endif +} + +/** Replacement for ftruncate(fd, 0): move to the front of the file and remove + * all the rest of the file. Return -1 on error, 0 on success. */ +int +tor_ftruncate(int fd) +{ + /* Rumor has it that some versions of ftruncate do not move the file pointer. + */ + if (tor_fd_setpos(fd, 0) < 0) + return -1; + +#ifdef _WIN32 + return _chsize(fd, 0); +#else + return ftruncate(fd, 0); +#endif +} + +/** Minimal version of write_all, for use by logging. */ +int +write_all_to_fd(int fd, const char *buf, size_t count) +{ + size_t written = 0; + raw_assert(count < SSIZE_MAX); + + while (written < count) { + ssize_t result = write(fd, buf+written, count-written); + if (result<0) + return -1; + written += result; + } + return 0; +} diff --git a/src/lib/fdio/fdio.h b/src/lib/fdio/fdio.h new file mode 100644 index 000000000..8cc4a0465 --- /dev/null +++ b/src/lib/fdio/fdio.h @@ -0,0 +1,17 @@ +/* Copyright (c) 2003-2004, Roger Dingledine + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#ifndef TOR_FDIO_H +#define TOR_FDIO_H + +#include <stddef.h> + +off_t tor_fd_getpos(int fd); +int tor_fd_setpos(int fd, off_t pos); +int tor_fd_seekend(int fd); +int tor_ftruncate(int fd); +int write_all_to_fd(int fd, const char *buf, size_t count); + +#endif /* !defined(TOR_FDIO_H) */ diff --git a/src/lib/fdio/include.am b/src/lib/fdio/include.am new file mode 100644 index 000000000..6c18f00a0 --- /dev/null +++ b/src/lib/fdio/include.am @@ -0,0 +1,17 @@ + +noinst_LIBRARIES += src/lib/libtor-fdio.a + +if UNITTESTS_ENABLED +noinst_LIBRARIES += src/lib/libtor-fdio-testing.a +endif + +src_lib_libtor_fdio_a_SOURCES = \ + src/lib/fdio/fdio.c + +src_lib_libtor_fdio_testing_a_SOURCES = \ + $(src_lib_libtor_fdio_a_SOURCES) +src_lib_libtor_fdio_testing_a_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS) +src_lib_libtor_fdio_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS) + +noinst_HEADERS += \ + src/lib/fdio/fdio.h diff --git a/src/lib/log/torlog.c b/src/lib/log/torlog.c index fc2f898d8..5709dd819 100644 --- a/src/lib/log/torlog.c +++ b/src/lib/log/torlog.c @@ -37,11 +37,14 @@ #include "lib/container/smartlist.h" #include "lib/err/torerr.h" #include "lib/intmath/bits.h" +#include "lib/string/compat_string.h" +#include "lib/string/printf.h" #include "lib/malloc/util_malloc.h" #include "lib/string/util_string.h" #include "lib/wallclock/tor_gettimeofday.h" #include "lib/wallclock/approx_time.h" #include "lib/wallclock/tm_cvt.h" +#include "lib/fdio/fdio.h"
#ifdef HAVE_ANDROID_LOG_H #include <android/log.h> @@ -201,12 +204,12 @@ static int pretty_fn_has_parens = 0;
/** Lock the log_mutex to prevent others from changing the logfile_t list */ #define LOCK_LOGS() STMT_BEGIN \ - tor_assert(log_mutex_initialized); \ + raw_assert(log_mutex_initialized); \ tor_mutex_acquire(&log_mutex); \ STMT_END /** Unlock the log_mutex */ #define UNLOCK_LOGS() STMT_BEGIN \ - tor_assert(log_mutex_initialized); \ + raw_assert(log_mutex_initialized); \ tor_mutex_release(&log_mutex); \ STMT_END
@@ -310,22 +313,6 @@ log_prefix_(char *buf, size_t buf_len, int severity) return n+r; }
-/** Minimal version of write_all, for use by logging. */ -static int -write_all_to_fd(int fd, const char *buf, size_t count) -{ - size_t written = 0; - raw_assert(count < SSIZE_MAX); - - while (written < count) { - ssize_t result = write(fd, buf+written, count-written); - if (result<0) - return -1; - written += result; - } - return 0; -} - /** If lf refers to an actual file that we have just opened, and the file * contains no data, log an "opening new logfile" message at the top. * @@ -596,7 +583,7 @@ logv,(int severity, log_domain_mask_t domain, const char *funcname, char *end_of_prefix=NULL; int callbacks_deferred = 0;
- /* Call assert, not tor_assert, since tor_assert calls log on failure. */ + /* Call assert, not raw_assert, since raw_assert calls log on failure. */ raw_assert(format); /* check that severity is sane. Overrunning the masks array leads to * interesting and hard to diagnose effects */ @@ -711,7 +698,7 @@ tor_log_update_sigsafe_err_fds(void) if (!found_real_stderr && int_array_contains(fds, n_fds, STDOUT_FILENO)) { /* Don't use a virtual stderr when we're also logging to stdout. */ - raw_assert(n_fds >= 2); /* Don't tor_assert inside log fns */ + raw_assert(n_fds >= 2); /* Don't raw_assert inside log fns */ fds[0] = fds[--n_fds]; }
@@ -726,7 +713,7 @@ void tor_log_get_logfile_names(smartlist_t *out) { logfile_t *lf; - tor_assert(out); + raw_assert(out);
LOCK_LOGS();
@@ -839,8 +826,8 @@ delete_log(logfile_t *victim) logfiles = victim->next; else { for (tmpl = logfiles; tmpl && tmpl->next != victim; tmpl=tmpl->next) ; -// tor_assert(tmpl); -// tor_assert(tmpl->next == victim); +// raw_assert(tmpl); +// raw_assert(tmpl->next == victim); if (!tmpl) return; tmpl->next = victim->next; @@ -874,9 +861,9 @@ set_log_severity_config(int loglevelMin, int loglevelMax, log_severity_list_t *severity_out) { int i; - tor_assert(loglevelMin >= loglevelMax); - tor_assert(loglevelMin >= LOG_ERR && loglevelMin <= LOG_DEBUG); - tor_assert(loglevelMax >= LOG_ERR && loglevelMax <= LOG_DEBUG); + raw_assert(loglevelMin >= loglevelMax); + raw_assert(loglevelMin >= LOG_ERR && loglevelMin <= LOG_DEBUG); + raw_assert(loglevelMax >= LOG_ERR && loglevelMax <= LOG_DEBUG); memset(severity_out, 0, sizeof(log_severity_list_t)); for (i = loglevelMin; i >= loglevelMax; --i) { severity_out->masks[SEVERITY_MASK_IDX(i)] = ~0u; @@ -1146,20 +1133,17 @@ mark_logs_temp(void) }
/** - * Add a log handler to send messages to <b>filename</b>. If opening the - * logfile fails, -1 is returned and errno is set appropriately (by open(2)). + * Add a log handler to send messages to <b>filename</b> via <b>fd</b>. If + * opening the logfile failed, -1 is returned and errno is set appropriately + * (by open(2)). Takes ownership of fd. */ int -add_file_log(const log_severity_list_t *severity, const char *filename, - const int truncate_log) +add_file_log(const log_severity_list_t *severity, + const char *filename, + int fd) { - int fd; logfile_t *lf;
- int open_flags = O_WRONLY|O_CREAT; - open_flags |= truncate_log ? O_TRUNC : O_APPEND; - - fd = tor_open_cloexec(filename, open_flags, 0640); if (fd<0) return -1; if (tor_fd_seekend(fd)<0) { diff --git a/src/lib/log/torlog.h b/src/lib/log/torlog.h index 6814cc9d0..c24b63819 100644 --- a/src/lib/log/torlog.h +++ b/src/lib/log/torlog.h @@ -145,8 +145,10 @@ void set_log_severity_config(int minSeverity, int maxSeverity, log_severity_list_t *severity_out); void add_stream_log(const log_severity_list_t *severity, const char *name, int fd); -int add_file_log(const log_severity_list_t *severity, const char *filename, - const int truncate); +int add_file_log(const log_severity_list_t *severity, + const char *filename, + int fd); + #ifdef HAVE_SYSLOG_H int add_syslog_log(const log_severity_list_t *severity, const char* syslog_identity_tag); diff --git a/src/or/config.c b/src/or/config.c index 6bdb4ab7d..cc3cc3ec5 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -5638,6 +5638,23 @@ addressmap_register_auto(const char *from, const char *to, }
/** + * As add_file_log, but open the file as appropriate. + */ +STATIC int +open_and_add_file_log(const log_severity_list_t *severity, + const char *filename, int truncate_log) +{ + int open_flags = O_WRONLY|O_CREAT; + open_flags |= truncate_log ? O_TRUNC : O_APPEND; + + int fd = tor_open_cloexec(filename, open_flags, 0640); + if (fd < 0) + return -1; + + return add_file_log(severity, filename, fd); +} + +/** * Initialize the logs based on the configuration file. */ static int @@ -5762,7 +5779,7 @@ options_init_logs(const or_options_t *old_options, or_options_t *options, } } } - if (add_file_log(severity, fname, truncate_log) < 0) { + if (open_and_add_file_log(severity, fname, truncate_log) < 0) { log_warn(LD_CONFIG, "Couldn't open file for 'Log %s': %s", opt->value, strerror(errno)); ok = 0; @@ -8452,4 +8469,3 @@ options_any_client_port_set(const or_options_t *options) options->DNSPort_set || options->HTTPTunnelPort_set); } - diff --git a/src/or/config.h b/src/or/config.h index dc3322e6b..d2faf6c51 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -271,8 +271,10 @@ STATIC int check_bridge_distribution_setting(const char *bd);
STATIC uint64_t compute_real_max_mem_in_queues(const uint64_t val, int log_guess); +STATIC int open_and_add_file_log(const log_severity_list_t *severity, + const char *fname, + int truncate_log);
#endif /* defined(CONFIG_PRIVATE) */
#endif /* !defined(TOR_CONFIG_H) */ - diff --git a/src/or/keypin.c b/src/or/keypin.c index 312530fe4..db267673f 100644 --- a/src/or/keypin.c +++ b/src/or/keypin.c @@ -20,6 +20,7 @@ #include "siphash.h" #include "lib/cc/torint.h" #include "lib/log/torlog.h" +#include "lib/fdio/fdio.h" #include "common/util.h" #include "common/util_format.h"
@@ -498,4 +499,3 @@ keypin_clear(void) bad_entries); } } - diff --git a/src/or/microdesc.c b/src/or/microdesc.c index d29d2c300..bbe5ead6b 100644 --- a/src/or/microdesc.c +++ b/src/or/microdesc.c @@ -9,6 +9,9 @@ */
#include "or/or.h" + +#include "lib/fdio/fdio.h" + #include "or/circuitbuild.h" #include "or/config.h" #include "or/directory.h" @@ -1047,4 +1050,3 @@ usable_consensus_flavor,(void)) return FLAV_NS; } } - diff --git a/src/test/test_logging.c b/src/test/test_logging.c index c92dd620f..06744ebf2 100644 --- a/src/test/test_logging.c +++ b/src/test/test_logging.c @@ -1,8 +1,11 @@ /* Copyright (c) 2013-2018, The Tor Project, Inc. */ /* See LICENSE for licensing information */
+#define CONFIG_PRIVATE + #include "orconfig.h" #include "or/or.h" +#include "or/config.h" #include "lib/err/torerr.h" #include "lib/log/torlog.h" #include "test/test.h" @@ -90,7 +93,7 @@ test_sigsafe_err(void *arg)
init_logging(1); mark_logs_temp(); - add_file_log(&include_bug, fn, 0); + open_and_add_file_log(&include_bug, fn, 0); tor_log_update_sigsafe_err_fds(); close_temp_logs();
diff --git a/src/test/test_util.c b/src/test/test_util.c index 69c1f3c84..73f353a3f 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -19,6 +19,7 @@ #include "common/util_process.h" #include "test/log_test_helpers.h" #include "lib/compress/compress_zstd.h" +#include "lib/fdio/fdio.h"
#ifdef HAVE_PWD_H #include <pwd.h>