[or-cvs] r17456: {tor} Change logging code to use fds instead of stdio. Fixes bug 8 (in tor/trunk: . src/common src/or src/tools)
nickm at seul.org
nickm at seul.org
Tue Dec 2 23:36:59 UTC 2008
Author: nickm
Date: 2008-12-02 18:36:58 -0500 (Tue, 02 Dec 2008)
New Revision: 17456
Modified:
tor/trunk/ChangeLog
tor/trunk/src/common/log.c
tor/trunk/src/common/log.h
tor/trunk/src/or/config.c
tor/trunk/src/or/test.c
tor/trunk/src/tools/tor-gencert.c
tor/trunk/src/tools/tor-resolve.c
Log:
Change logging code to use fds instead of stdio. Fixes bug 861, and probably makes logging slightly faster. Not a backport candidate: bug 861 is too obscure and harmless.
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2008-12-02 23:33:55 UTC (rev 17455)
+++ tor/trunk/ChangeLog 2008-12-02 23:36:58 UTC (rev 17456)
@@ -26,6 +26,9 @@
- Build correctly on platforms without socklen_t.
- Avoid potential crash on internal error during signature collection.
Fixes bug 864. Patch from rovv.
+ - Do not use C's stdio library for writing to log files. This will
+ improve logging performance by a minute amount, and will stop leaking
+ fds when our disk is full. Fixes bug 861.
o Minor features:
- Report the case where all signatures in a detached set are rejected
Modified: tor/trunk/src/common/log.c
===================================================================
--- tor/trunk/src/common/log.c 2008-12-02 23:33:55 UTC (rev 17455)
+++ tor/trunk/src/common/log.c 2008-12-02 23:36:58 UTC (rev 17456)
@@ -14,7 +14,7 @@
#include "orconfig.h"
#include <stdarg.h>
#include <assert.h>
-#include <stdio.h>
+// #include <stdio.h>
#include <stdlib.h>
#include <string.h>
#ifdef HAVE_SYS_TIME_H
@@ -23,6 +23,16 @@
#ifdef HAVE_TIME_H
#include <time.h>
#endif
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+#ifdef HAVE_FCNTL_H
+#include <fcntl.h>
+#endif
+#include "compat.h"
#include "util.h"
#define LOG_PRIVATE
#include "log.h"
@@ -37,7 +47,7 @@
typedef struct logfile_t {
struct logfile_t *next; /**< Next logfile_t in the linked list. */
char *filename; /**< Filename to open. */
- FILE *file; /**< Stream to receive log messages. */
+ int fd; /**< fd to receive log messages, or -1 for none. */
int seems_dead; /**< Boolean: true if the stream seems to be kaput. */
int needs_close; /**< Boolean: true if the stream gets closed on shutdown. */
int is_temporary; /**< Boolean: close after initializing logging subsystem.*/
@@ -170,11 +180,9 @@
if (lf->is_temporary)
/* If it's temporary, it isn't really a file. */
return 0;
-#ifdef HAVE_FTELLO
- is_new = (ftello(lf->file) == 0);
-#else
- is_new = (ftell(lf->file) == 0);
-#endif
+
+ is_new = lf->fd >= 0 && tor_fd_getpos(lf->fd) == 0;
+
if (reset && !is_new)
/* We are resetting, but we aren't at the start of the file; no
* need to log again. */
@@ -187,8 +195,7 @@
tor_snprintf(buf+n, sizeof(buf)-n,
"Tor %s opening %slog file.\n", VERSION, is_new?"new ":"");
}
- if (fputs(buf, lf->file) == EOF ||
- fflush(lf->file) == EOF) /* error */
+ if (write_all(lf->fd, buf, strlen(buf), 0) < 0) /* error */
return -1; /* failed */
return 0;
}
@@ -201,7 +208,7 @@
static INLINE char *
format_msg(char *buf, size_t buf_len,
log_domain_mask_t domain, int severity, const char *funcname,
- const char *format, va_list ap)
+ const char *format, va_list ap, size_t *msg_len_out)
{
size_t n;
int r;
@@ -244,6 +251,7 @@
}
buf[n]='\n';
buf[n+1]='\0';
+ *msg_len_out = n+1;
return end_of_prefix;
}
@@ -256,6 +264,7 @@
const char *format, va_list ap)
{
char buf[10024];
+ size_t msg_len = 0;
int formatted = 0;
logfile_t *lf;
char *end_of_prefix=NULL;
@@ -272,7 +281,7 @@
lf = lf->next;
continue;
}
- if (! (lf->file || lf->is_syslog || lf->callback)) {
+ if (! (lf->fd >= 0 || lf->is_syslog || lf->callback)) {
lf = lf->next;
continue;
}
@@ -283,7 +292,8 @@
if (!formatted) {
end_of_prefix =
- format_msg(buf, sizeof(buf), domain, severity, funcname, format, ap);
+ format_msg(buf, sizeof(buf), domain, severity, funcname, format, ap,
+ &msg_len);
formatted = 1;
}
if (lf->is_syslog) {
@@ -299,8 +309,7 @@
lf = lf->next;
continue;
}
- if (fputs(buf, lf->file) == EOF ||
- fflush(lf->file) == EOF) { /* error */
+ if (write_all(lf->fd, buf, msg_len, 0) < 0) { /* error */
/* don't log the error! mark this log entry to be blown away, and
* continue. */
lf->seems_dead = 1;
@@ -465,8 +474,9 @@
static void
close_log(logfile_t *victim)
{
- if (victim->needs_close && victim->file) {
- fclose(victim->file);
+ if (victim->needs_close && victim->fd >= 0) {
+ close(victim->fd);
+ victim->fd = -1;
} else if (victim->is_syslog) {
#ifdef HAVE_SYSLOG_H
if (--syslog_count == 0) {
@@ -495,16 +505,16 @@
}
/** Add a log handler named <b>name</b> to send all messages in <b>severity</b>
- * to <b>stream</b>. Copies <b>severity</b>. Helper: does no locking. */
+ * to <b>fd</b>. Copies <b>severity</b>. Helper: does no locking. */
static void
add_stream_log_impl(const log_severity_list_t *severity,
- const char *name, FILE *stream)
+ const char *name, int fd)
{
logfile_t *lf;
lf = tor_malloc_zero(sizeof(logfile_t));
+ lf->fd = fd;
lf->filename = tor_strdup(name);
lf->severities = tor_memdup(severity, sizeof(log_severity_list_t));
- lf->file = stream;
lf->next = logfiles;
logfiles = lf;
@@ -512,14 +522,14 @@
}
/** Add a log handler named <b>name</b> to send all messages in <b>severity</b>
- * to <b>stream</b>. Steals a reference to <b>severity</b>; the caller must
+ * to <b>fd</b>. Steals a reference to <b>severity</b>; the caller must
* not use it after calling this function. */
void
add_stream_log(const log_severity_list_t *severity,
- const char *name, FILE *stream)
+ const char *name, int fd)
{
LOCK_LOGS();
- add_stream_log_impl(severity, name, stream);
+ add_stream_log_impl(severity, name, fd);
UNLOCK_LOGS();
}
@@ -540,7 +550,7 @@
log_severity_list_t *s = tor_malloc_zero(sizeof(log_severity_list_t));
set_log_severity_config(min_severity, LOG_ERR, s);
LOCK_LOGS();
- add_stream_log_impl(s, "<temp>", stdout);
+ add_stream_log_impl(s, "<temp>", fileno(stdout));
tor_free(s);
logfiles->is_temporary = 1;
UNLOCK_LOGS();
@@ -555,6 +565,7 @@
{
logfile_t *lf;
lf = tor_malloc_zero(sizeof(logfile_t));
+ lf->fd = -1;
lf->severities = tor_memdup(severity, sizeof(log_severity_list_t));
lf->filename = tor_strdup("<callback>");
lf->callback = cb;
@@ -635,19 +646,23 @@
}
/**
- * 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 fopen).
+ * 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)).
*/
int
add_file_log(const log_severity_list_t *severity, const char *filename)
{
- FILE *f;
+ int fd;
logfile_t *lf;
- f = fopen(filename, "a");
- if (!f) return -1;
+
+ fd = open(filename, O_WRONLY|O_CREAT|O_APPEND, 0644);
+ if (fd<0)
+ return -1;
+ if (tor_fd_seekend(fd)<0)
+ return -1;
+
LOCK_LOGS();
- add_stream_log_impl(severity, filename, f);
+ add_stream_log_impl(severity, filename, fd);
logfiles->needs_close = 1;
lf = logfiles;
_log_global_min_severity = get_min_log_level();
@@ -675,6 +690,7 @@
openlog("Tor", LOG_PID | LOG_NDELAY, LOGFACILITY);
lf = tor_malloc_zero(sizeof(logfile_t));
+ lf->fd = -1;
lf->severities = tor_memdup(severity, sizeof(log_severity_list_t));
lf->filename = tor_strdup("<syslog>");
Modified: tor/trunk/src/common/log.h
===================================================================
--- tor/trunk/src/common/log.h 2008-12-02 23:33:55 UTC (rev 17455)
+++ tor/trunk/src/common/log.h 2008-12-02 23:36:58 UTC (rev 17456)
@@ -123,7 +123,7 @@
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,
- FILE *stream);
+ int fd);
int add_file_log(const log_severity_list_t *severity, const char *filename);
#ifdef HAVE_SYSLOG_H
int add_syslog_log(const log_severity_list_t *severity);
Modified: tor/trunk/src/or/config.c
===================================================================
--- tor/trunk/src/or/config.c 2008-12-02 23:33:55 UTC (rev 17455)
+++ tor/trunk/src/or/config.c 2008-12-02 23:36:58 UTC (rev 17456)
@@ -4170,7 +4170,7 @@
err?"stderr":"stdout");
} else {
add_stream_log(severity, err?"<stderr>":"<stdout>",
- err?stderr:stdout);
+ fileno(err?stderr:stdout));
}
}
goto cleanup;
Modified: tor/trunk/src/or/test.c
===================================================================
--- tor/trunk/src/or/test.c 2008-12-02 23:33:55 UTC (rev 17455)
+++ tor/trunk/src/or/test.c 2008-12-02 23:36:58 UTC (rev 17456)
@@ -4479,7 +4479,7 @@
log_severity_list_t s;
memset(&s, 0, sizeof(s));
set_log_severity_config(loglevel, LOG_ERR, &s);
- add_stream_log(&s, "", stdout);
+ add_stream_log(&s, "", fileno(stdout));
}
options->command = CMD_RUN_UNITTESTS;
Modified: tor/trunk/src/tools/tor-gencert.c
===================================================================
--- tor/trunk/src/tools/tor-gencert.c 2008-12-02 23:33:55 UTC (rev 17455)
+++ tor/trunk/src/tools/tor-gencert.c 2008-12-02 23:36:58 UTC (rev 17456)
@@ -195,7 +195,7 @@
set_log_severity_config(LOG_DEBUG, LOG_ERR, &s);
else
set_log_severity_config(LOG_WARN, LOG_ERR, &s);
- add_stream_log(&s, "<stderr>", stderr);
+ add_stream_log(&s, "<stderr>", fileno(stderr));
if (!identity_key_file) {
identity_key_file = tor_strdup("./authority_identity_key");
Modified: tor/trunk/src/tools/tor-resolve.c
===================================================================
--- tor/trunk/src/tools/tor-resolve.c 2008-12-02 23:33:55 UTC (rev 17455)
+++ tor/trunk/src/tools/tor-resolve.c 2008-12-02 23:36:58 UTC (rev 17456)
@@ -367,7 +367,7 @@
set_log_severity_config(LOG_DEBUG, LOG_ERR, s);
else
set_log_severity_config(LOG_WARN, LOG_ERR, s);
- add_stream_log(s, "<stderr>", stderr);
+ add_stream_log(s, "<stderr>", fileno(stderr));
if (n_args == 1) {
log_debug(LD_CONFIG, "defaulting to localhost");
More information about the tor-commits
mailing list