commit 79f73ab330c0e643ba1ec2eb0633f0f80525a083
Author: Nick Mathewson <nickm(a)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>
