From 142f411b1305d4c9857532f66e61a895604160a9 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 16 Apr 2012 00:25:46 +0200 Subject: Improve string escape performance a little Instead of escaping a string, returning a new one and running strlen() on that, use an output variable to store the length, since we can easily calculate that during the escape process anyway. This should shave off a tiny bit of CPU time. Signed-off-by: Gergely Nagy --- lib/buffer.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/buffer.c b/lib/buffer.c index 8bdb3ef..3d92224 100644 --- a/lib/buffer.c +++ b/lib/buffer.c @@ -55,7 +55,7 @@ static const unsigned char json_exceptions[] = }; static inline char * -_ul_str_escape (const char *str) +_ul_str_escape (const char *str, size_t *length) { const unsigned char *p; char *dest; @@ -139,6 +139,8 @@ _ul_str_escape (const char *str) } *q = 0; + if (length) + *length = q - dest; return dest; } @@ -170,19 +172,16 @@ ul_buffer_append (ul_buffer_t *buffer, const char *key, const char *value) char *k, *v; size_t lk, lv; - k = _ul_str_escape (key); + k = _ul_str_escape (key, &lk); if (!k) return NULL; - v = _ul_str_escape (value); + v = _ul_str_escape (value, &lv); if (!v) { free (k); return NULL; } - lk = strlen (k); - lv = strlen (v); - buffer = _ul_buffer_ensure_size (buffer, buffer->len + lk + lv + 6); if (!buffer) { -- cgit From c568a2c036ee555faa7b24cdfe07d65af0a15361 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 16 Apr 2012 00:36:37 +0200 Subject: Free up the JSON buffer on exit This adds a destructor that simply frees up the JSON buffer, so that to not upset valgrind and similar tools too much. Signed-off-by: Gergely Nagy --- lib/umberlog.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'lib') diff --git a/lib/umberlog.c b/lib/umberlog.c index fe0a6b7..84bd64b 100644 --- a/lib/umberlog.c +++ b/lib/umberlog.c @@ -51,6 +51,7 @@ static void (*old_openlog) (); static int (*old_setlogmask) (); static void ul_init (void) __attribute__((constructor)); +static void ul_finish (void) __attribute__((destructor)); static __thread struct { @@ -77,6 +78,12 @@ ul_init (void) old_setlogmask = dlsym (RTLD_NEXT, "setlogmask"); } +static void +ul_finish (void) +{ + free (ul_buffer.msg); +} + void ul_openlog (const char *ident, int option, int facility) { -- cgit From 32887a13759b27f5b29f585d2b278c225be1fa0d Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Fri, 20 Apr 2012 12:37:35 +0200 Subject: Properly reset the JSON buffer in all cases. Previously the JSON buffer was only reset when ul_vformat() was called, but not everything goes through that, so some messages ended up being garbled. The proper thing to do is to reset in _ul_vformat(), as everything goes through that, and that's the right place to do this anyway. Reported-by: Peter Czanik Signed-off-by: Gergely Nagy --- lib/umberlog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/umberlog.c b/lib/umberlog.c index 84bd64b..d07a24a 100644 --- a/lib/umberlog.c +++ b/lib/umberlog.c @@ -348,6 +348,8 @@ _ul_vformat (ul_buffer_t *buffer, int format_version, if (!value) return NULL; + ul_buffer_reset (buffer); + buffer = ul_buffer_append (buffer, "msg", value); if (buffer == NULL) { @@ -402,8 +404,6 @@ ul_vformat (int priority, const char *msg_format, va_list ap) const char *msg; ul_buffer_t *buffer = &ul_buffer; - ul_buffer_reset (buffer); - msg = _ul_vformat_str (buffer, 1, priority, msg_format, ap); if (!msg) { -- cgit From 5509ee80130cb781d0c88d411021a98b5fab4556 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Fri, 20 Apr 2012 13:14:29 +0200 Subject: Resolve facility & priority properly The priority passed to the syslog() call can contain a facility embedded, so we need to extract it to find both the facility and the priority. This also means that the facility set at openlog()-time can be overridden later, so _find_facility() has to take the priority value into account. This patch makes libumberlog do just the above: extract both priority and facility from the priority passed to syslog(), and if facility is 0, use the default set at openlog()-time, if any. Added a test case to test this expected behaviour. Reported-by: Peter Czanik Signed-off-by: Gergely Nagy --- lib/umberlog.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/umberlog.c b/lib/umberlog.c index d07a24a..ad25709 100644 --- a/lib/umberlog.c +++ b/lib/umberlog.c @@ -101,15 +101,19 @@ ul_openlog (const char *ident, int option, int facility) /** HELPERS **/ static inline const char * -_find_facility (void) +_find_facility (int prio) { int i = 0; + int fac = prio & LOG_FACMASK; + + if (fac == 0) + fac = ul_sys_settings.facility; while (facilitynames[i].c_name != NULL && - facilitynames[i].c_val != ul_sys_settings.facility) + facilitynames[i].c_val != fac) i++; - if (facilitynames[i].c_val == ul_sys_settings.facility) + if (facilitynames[i].c_val == fac) return facilitynames[i].c_name; return ""; } @@ -118,12 +122,13 @@ static inline const char * _find_prio (int prio) { int i = 0; + int pri = LOG_PRI (prio); while (prioritynames[i].c_name != NULL && - prioritynames[i].c_val != prio) + prioritynames[i].c_val != pri) i++; - if (prioritynames[i].c_val == prio) + if (prioritynames[i].c_val == pri) return prioritynames[i].c_name; return ""; } @@ -316,7 +321,7 @@ _ul_discover (ul_buffer_t *buffer, int priority) buffer = _ul_json_append (buffer, "pid", "%d", _find_pid (), - "facility", "%s", _find_facility (), + "facility", "%s", _find_facility (priority), "priority", "%s", _find_prio (priority), "program", "%s", ul_sys_settings.ident, "uid", "%d", _get_uid (), -- cgit From 8cfef10830350c565e8ee917f83b9e97016a0a62 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sat, 28 Apr 2012 18:18:32 +0200 Subject: Add a closelog() wrapper to clear the environment Our openlog() wrapper fills in a couple of variables, and those were kept around even after a closelog(), and thus, affected ul_format() calls even after a closelog(). This in turn, made one of the test cases fail, as that was relying on the default behaviour, which was modified due to an openlog() in an earlier test case. We now wrap closelog() aswell, and NULL out our settings to get a clean state, and add a test case to verify this behaviour aswell. Signed-off-by: Gergely Nagy --- lib/libumberlog.ld | 5 +++++ lib/umberlog.c | 12 ++++++++++++ lib/umberlog.h | 1 + lib/umberlog.rst | 8 +++++++- 4 files changed, 25 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/libumberlog.ld b/lib/libumberlog.ld index 8cd48e7..225d51c 100644 --- a/lib/libumberlog.ld +++ b/lib/libumberlog.ld @@ -15,3 +15,8 @@ LIBUMBERLOG_0.1.0 { facilitynames; prioritynames; }; + +LIBUMBERLOG_0.2.1 { + global: + ul_closelog; +} LIBUMBERLOG_0.1.0; diff --git a/lib/umberlog.c b/lib/umberlog.c index ad25709..e4eb5d3 100644 --- a/lib/umberlog.c +++ b/lib/umberlog.c @@ -48,6 +48,7 @@ static void (*old_syslog) (); static void (*old_vsyslog) (); static void (*old_openlog) (); +static void (*old_closelog) (); static int (*old_setlogmask) (); static void ul_init (void) __attribute__((constructor)); @@ -75,6 +76,7 @@ ul_init (void) old_syslog = dlsym (RTLD_NEXT, "syslog"); old_vsyslog = dlsym (RTLD_NEXT, "vsyslog"); old_openlog = dlsym (RTLD_NEXT, "openlog"); + old_closelog = dlsym (RTLD_NEXT, "closelog"); old_setlogmask = dlsym (RTLD_NEXT, "setlogmask"); } @@ -99,6 +101,13 @@ ul_openlog (const char *ident, int option, int facility) gethostname (ul_sys_settings.hostname, _POSIX_HOST_NAME_MAX); } +void +ul_closelog (void) +{ + old_closelog (); + memset (&ul_sys_settings, 0, sizeof (ul_sys_settings)); +} + /** HELPERS **/ static inline const char * _find_facility (int prio) @@ -512,6 +521,9 @@ __vsyslog_chk (int __pri, int __flag, __const char *__fmt, va_list ap) void openlog (const char *ident, int option, int facility) __attribute__((alias ("ul_openlog"))); +void closelog (void) + __attribute__((alias ("ul_closelog"))); + #undef syslog void syslog (int priority, const char *msg_format, ...) __attribute__((alias ("ul_legacy_syslog"))); diff --git a/lib/umberlog.h b/lib/umberlog.h index 4245dae..51ed7be 100644 --- a/lib/umberlog.h +++ b/lib/umberlog.h @@ -41,6 +41,7 @@ char *ul_format (int priority, const char *msg_format, ...) char *ul_vformat (int priority, const char *msg_format, va_list ap); void ul_openlog (const char *ident, int option, int facility); +void ul_closelog (void); int ul_setlogmask (int mask); int ul_syslog (int priority, const char *msg_format, ...) diff --git a/lib/umberlog.rst b/lib/umberlog.rst index c655263..d2cd354 100644 --- a/lib/umberlog.rst +++ b/lib/umberlog.rst @@ -7,7 +7,7 @@ CEE-enhanced syslog message generation -------------------------------------- :Author: Gergely Nagy -:Date: 2012-03-23 +:Date: 2012-04-28 :Manual section: 3 :Manual group: CEE-enhanced syslog Manual @@ -19,6 +19,7 @@ SYNOPSIS #include void ul_openlog (const char *ident, int option, int facility); + void ul_closelog (void); int ul_syslog (int priority, const char *format, ....); int ul_vsyslog (int priority, const char *format, va_list ap); @@ -37,6 +38,11 @@ the original **openlog()** function, which opens a connection to the system logger for a program. The updated version adds support for a number of new option flags, described below. +**ul_closelog()** (also aliased to **closelog()**) is similar to +**ul_openlog()** in that it is a wrapper around the original +**closelog()**. It clears any settings set so far, to get back to a +clean state. + **ul_legacy_syslog()** and **ul_legacy_vsyslog()** are both thin layers over the original **syslog()** and **vsyslog()** functions, and the library overrides the original functions with this two. The only -- cgit From cf66dbeb8820ef935045813598aa5a48be00526a Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sat, 28 Apr 2012 18:47:43 +0200 Subject: Preparations for 0.2.1 Bump software and library interface versions, and start adding NEWS entries. Signed-off-by: Gergely Nagy --- lib/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/Makefile.am b/lib/Makefile.am index 169b0b2..b113810 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -1,6 +1,6 @@ -LUL_CURRENT = 1 +LUL_CURRENT = 2 LUL_REVISION = 0 -LUL_AGE = 0 +LUL_AGE = 1 lib_LTLIBRARIES = libumberlog.la libumberlog_la_LDFLAGS = -Wl,--version-script,${srcdir}/libumberlog.ld \ -- cgit From 7dc0d11639d5f9fd2c4d0e01ab12813a5898f596 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sun, 29 Apr 2012 10:34:04 +0200 Subject: Generate the manual page if rst2man is present If rst2man is present at configure time, generate the manual page with it, which is to be included in the dist tarball too. Signed-off-by: Gergely Nagy --- lib/Makefile.am | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'lib') diff --git a/lib/Makefile.am b/lib/Makefile.am index b113810..b6b60b3 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -15,3 +15,12 @@ pkgconfigdir = $(libdir)/pkgconfig pkgconfig_DATA = libumberlog.pc EXTRA_DIST = umberlog.rst libumberlog.ld + +if ENABLE_MANS +man3_MANS = umberlog.3 +CLEANFILES = umberlog.3 +EXTRA_DIST += umberlog.3 + +umberlog.3: umberlog.rst + $(AM_V_GEN) $(RST2MAN) $< $@ +endif -- cgit From 4fd758dcde350fda5e87573d9c7004a35b136741 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sun, 29 Apr 2012 11:12:39 +0200 Subject: Reduce the number of memory allocations Make the string escape use an ul_buffer_t, to avoid unnecessary memory allocations. This greatly reduces the number of mallocs made, and thus results in a noticable increase in speed. Signed-off-by: Gergely Nagy --- lib/buffer.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/buffer.c b/lib/buffer.c index 3d92224..ef448bf 100644 --- a/lib/buffer.c +++ b/lib/buffer.c @@ -54,11 +54,20 @@ static const unsigned char json_exceptions[] = 0xff, '\0' }; +static __thread ul_buffer_t escape_buffer; + +static void ul_buffer_finish (void) __attribute__((destructor)); + +static void +ul_buffer_finish (void) +{ + free (escape_buffer.msg); +} + static inline char * -_ul_str_escape (const char *str, size_t *length) +_ul_str_escape (const char *str, char *dest, size_t *length) { const unsigned char *p; - char *dest; char *q; static unsigned char exmap[256]; static int exmap_inited; @@ -67,7 +76,7 @@ _ul_str_escape (const char *str, size_t *length) return NULL; p = (unsigned char *)str; - q = dest = malloc (strlen (str) * 6 + 1); + q = dest; if (!exmap_inited) { @@ -171,35 +180,44 @@ ul_buffer_append (ul_buffer_t *buffer, const char *key, const char *value) { char *k, *v; size_t lk, lv; + size_t orig_len = buffer->len; - k = _ul_str_escape (key, &lk); + /* Append the key to the buffer */ + escape_buffer.len = 0; + _ul_buffer_ensure_size (&escape_buffer, strlen (key) * 6 + 1); + k = _ul_str_escape (key, escape_buffer.msg, &lk); if (!k) return NULL; - v = _ul_str_escape (value, &lv); + + buffer = _ul_buffer_ensure_size (buffer, buffer->len + lk + 4); + if (!buffer) + return NULL; + + memcpy (buffer->msg + buffer->len, "\"", 1); + memcpy (buffer->msg + buffer->len + 1, k, lk); + memcpy (buffer->msg + buffer->len + 1 + lk, "\":\"", 3); + + /* Append the value to the buffer */ + escape_buffer.len = 0; + _ul_buffer_ensure_size (&escape_buffer, strlen (value) * 6 + 1); + v = _ul_str_escape (value, escape_buffer.msg, &lv); if (!v) { - free (k); + buffer->len = orig_len; return NULL; } buffer = _ul_buffer_ensure_size (buffer, buffer->len + lk + lv + 6); if (!buffer) { - free (k); - free (v); + buffer->len = orig_len; return NULL; } - memcpy (buffer->msg + buffer->len, "\"", 1); - memcpy (buffer->msg + buffer->len + 1, k, lk); - memcpy (buffer->msg + buffer->len + 1 + lk, "\":\"", 3); memcpy (buffer->msg + buffer->len + 1 + lk + 3, v, lv); memcpy (buffer->msg + buffer->len + 1 + lk + 3 + lv, "\",", 2); buffer->len += lk + lv + 6; - free (k); - free (v); - return buffer; } -- cgit