From 018fbfa5f2ce139f893ff2076073d4966fda3f66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 2 Feb 2016 20:50:22 +0100 Subject: [PATCH 01/33] spec cleanup: get rid of redundant defattr References (first one is a direct trigger): https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/KEO7AX3JXR2TY6OVL4M7HDISZ6YIJNKU/#2UFET77NHMJTG4NA2ECRVH2KO3W56ZWD http://rpm.org/gitweb?p=rpm.git;a=commit;h=47ea5da (~2004) --- libqb.spec.in | 2 -- 1 file changed, 2 deletions(-) diff --git a/libqb.spec.in b/libqb.spec.in index 1e42ac593..10ef49067 100644 --- a/libqb.spec.in +++ b/libqb.spec.in @@ -45,7 +45,6 @@ rm -rf $RPM_BUILD_ROOT %postun -p /sbin/ldconfig %files -%defattr(-,root,root,-) %doc COPYING %{_sbindir}/qb-blackbox %{_libdir}/libqb.so.* @@ -60,7 +59,6 @@ The %{name}-devel package contains libraries and header files for developing applications that use %{name}. %files devel -%defattr(-,root,root,-) %doc COPYING README.markdown %{_includedir}/qb/ %{_libdir}/libqb.so From 11a5cea3a65099103596e73874ff1704c3004463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 2 Feb 2016 21:03:45 +0100 Subject: [PATCH 02/33] spec cleanup: remove ignored BuildRoot tag References: http://rpm.org/gitweb?p=rpm.git;a=commit;h=6c06519 (~2008) --- libqb.spec.in | 1 - 1 file changed, 1 deletion(-) diff --git a/libqb.spec.in b/libqb.spec.in index 10ef49067..fd226dbb1 100644 --- a/libqb.spec.in +++ b/libqb.spec.in @@ -11,7 +11,6 @@ Group: System Environment/Libraries License: LGPLv2+ URL: https://github.com/ClusterLabs/libqb Source0: https://fedorahosted.org/releases/q/u/quarterback/%{name}-%{version}%{?numcomm:.%{numcomm}}%{?alphatag:-%{alphatag}}%{?dirty:-%{dirty}}.tar.gz -BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildRequires: autoconf automake libtool doxygen procps check-devel From b7108edaff3ef12e464a7261cc7c3650457bf245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 2 Feb 2016 21:05:38 +0100 Subject: [PATCH 03/33] spec cleanup: get rid of redundant %clean section References: http://rpm.org/gitweb?p=rpm.git;a=commit;h=3fc5824 (~2009) --- libqb.spec.in | 3 --- 1 file changed, 3 deletions(-) diff --git a/libqb.spec.in b/libqb.spec.in index fd226dbb1..8640e8c70 100644 --- a/libqb.spec.in +++ b/libqb.spec.in @@ -36,9 +36,6 @@ make install DESTDIR=$RPM_BUILD_ROOT find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';' rm -rf $RPM_BUILD_ROOT/%{_datadir}/doc/libqb -%clean -rm -rf $RPM_BUILD_ROOT - %post -p /sbin/ldconfig %postun -p /sbin/ldconfig From ea35cfaa616096dd1d770fbda5a735108e1725f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Wed, 3 Feb 2016 15:30:39 +0100 Subject: [PATCH 04/33] build: avoid too keen -Wsuggest-attribute=format warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With gcc 5.3.1 20151207: > log.c: In function 'cs_format': > log.c:182:2: warning: function might be possible candidate for > 'gnu_printf' format attribute [-Wsuggest-attribute=format] > len = vsnprintf(str, QB_LOG_MAX_LEN, cs->format, ap_copy); > ^ We certainly don't want to disable that warning globally so make use of diagnostic pragmas for GCC instead in one instance that we cannot annotate properly. Signed-off-by: Jan Pokorný --- configure.ac | 6 ++++++ lib/log.c | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/configure.ac b/configure.ac index b8742e0ce..50e154941 100644 --- a/configure.ac +++ b/configure.ac @@ -488,6 +488,12 @@ for j in $WARNLIST; do fi done +# warnings suppression +if test x"$GCC" = xyes && cc_supports_flag -Wsuggest-attribute=format; then + AC_DEFINE([HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT], [], + [gcc supports -Wsuggest-attribute=format]) +fi + # --- coverage --- if test "x${enable_coverage}" = xyes && \ cc_supports_flag -ftest-coverage && \ diff --git a/lib/log.c b/lib/log.c index 3336af959..b6eb3ce6c 100644 --- a/lib/log.c +++ b/lib/log.c @@ -172,6 +172,12 @@ _cs_matches_filter_(struct qb_log_callsite *cs, * @param[in] cs Callsite containing format to use * @param[in] ap Variable arguments for format */ +#ifdef HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT +/* suppress suggestion that we currently can do nothing better about + as the format specification is hidden in cs argument */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wsuggest-attribute=format" +#endif static inline void cs_format(char *str, struct qb_log_callsite *cs, va_list ap) { @@ -188,6 +194,9 @@ cs_format(char *str, struct qb_log_callsite *cs, va_list ap) str[len - 1] = '\0'; } } +#ifdef HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT +#pragma GCC diagnostic pop +#endif void qb_log_real_va_(struct qb_log_callsite *cs, va_list ap) From 736e2c8153aa93025491849fa956fa7a3124d495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Wed, 3 Feb 2016 15:36:55 +0100 Subject: [PATCH 05/33] includes: format __attribute__ func. annotations in qblog.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan Pokorný --- include/qb/qblog.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/qb/qblog.h b/include/qb/qblog.h index 042d7a03f..870cbae79 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -296,7 +296,8 @@ void qb_log_from_external_source(const char *function, uint8_t priority, uint32_t lineno, uint32_t tags, - ...); + ...) + __attribute__ ((format (printf, 3, 7))); /** * Get or create a callsite at the give position. @@ -323,7 +324,8 @@ void qb_log_from_external_source_va(const char *function, uint8_t priority, uint32_t lineno, uint32_t tags, - va_list ap); + va_list ap) + __attribute__ ((format (printf, 3, 0))); /** * This is the function to generate a log message if you want to From 4037a3684dd538b3524ba9ed8ac5cae5502d9db0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 2 Feb 2016 21:09:02 +0100 Subject: [PATCH 06/33] spec cleanup: summary should not end with a dot References: https://fedoraproject.org/wiki/Common_Rpmlint_issues#summary-ended-with-dot https://en.opensuse.org/openSUSE:Specfile_guidelines#Summary + it's an internal inconsistency with Summary for -devel package --- libqb.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libqb.spec.in b/libqb.spec.in index 8640e8c70..7cba1eb7f 100644 --- a/libqb.spec.in +++ b/libqb.spec.in @@ -5,7 +5,7 @@ Name: libqb Version: @version@ Release: 1%{?numcomm:.%{numcomm}}%{?alphatag:.%{alphatag}}%{?dirty:.%{dirty}}%{?dist} -Summary: An IPC library for high performance servers. +Summary: An IPC library for high performance servers Group: System Environment/Libraries License: LGPLv2+ From 4fd4e5e71b7a04fd1ba9bb4a14d5c9d6bef796fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Wed, 3 Feb 2016 09:54:52 +0100 Subject: [PATCH 07/33] spec cleanup: drop redundant %{buildroot} cleaning References: http://pkgs.fedoraproject.org/cgit/rpms/redhat-rpm-config.git/commit/?id=159a65f (~2010) --- libqb.spec.in | 1 - 1 file changed, 1 deletion(-) diff --git a/libqb.spec.in b/libqb.spec.in index 7cba1eb7f..43cb0601e 100644 --- a/libqb.spec.in +++ b/libqb.spec.in @@ -31,7 +31,6 @@ make %{?_smp_mflags} make check %install -rm -rf $RPM_BUILD_ROOT make install DESTDIR=$RPM_BUILD_ROOT find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';' rm -rf $RPM_BUILD_ROOT/%{_datadir}/doc/libqb From 4f6c23547044eaf6cb03470c4bfeea8f6d6a1ced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Fri, 12 Feb 2016 23:33:13 +0100 Subject: [PATCH 08/33] build: avoid too keen -Wmissing-format-attribute warning Grouped with pre-existing -Wsuggest-attribute=format treatment. --- configure.ac | 11 +++++++++++ lib/log.c | 9 +++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 50e154941..da123d635 100644 --- a/configure.ac +++ b/configure.ac @@ -489,10 +489,21 @@ for j in $WARNLIST; do done # warnings suppression +gcc_format_complaints=no +if test x"$GCC" = xyes && cc_supports_flag -Wmissing-format-attribute; then + gcc_format_complaints=yes + AC_DEFINE([HAVE_GCC_MISSING_FORMAT_ATTRIBUTE], [], + [gcc supports -Wmissing-format-attribute]) +fi if test x"$GCC" = xyes && cc_supports_flag -Wsuggest-attribute=format; then + gcc_format_complaints=yes AC_DEFINE([HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT], [], [gcc supports -Wsuggest-attribute=format]) fi +if test x"$gcc_format_complaints" = xyes; then + AC_DEFINE([HAVE_GCC_FORMAT_COMPLAINTS], [], + [gcc can complain about missing format attribute]) +fi # --- coverage --- if test "x${enable_coverage}" = xyes && \ diff --git a/lib/log.c b/lib/log.c index b6eb3ce6c..459404672 100644 --- a/lib/log.c +++ b/lib/log.c @@ -172,12 +172,17 @@ _cs_matches_filter_(struct qb_log_callsite *cs, * @param[in] cs Callsite containing format to use * @param[in] ap Variable arguments for format */ -#ifdef HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT /* suppress suggestion that we currently can do nothing better about as the format specification is hidden in cs argument */ +#ifdef HAVE_GCC_FORMAT_COMPLAINTS #pragma GCC diagnostic push +#ifdef HAVE_GCC_MISSING_FORMAT_ATTRIBUTE +#pragma GCC diagnostic ignored "-Wmissing-format-attribute" +#endif +#ifdef HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT #pragma GCC diagnostic ignored "-Wsuggest-attribute=format" #endif +#endif static inline void cs_format(char *str, struct qb_log_callsite *cs, va_list ap) { @@ -194,7 +199,7 @@ cs_format(char *str, struct qb_log_callsite *cs, va_list ap) str[len - 1] = '\0'; } } -#ifdef HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT +#ifdef HAVE_GCC_FORMAT_COMPLAINTS #pragma GCC diagnostic pop #endif From e788d618ee55bf7c71284b3343dbe151ac777208 Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Sat, 13 Feb 2016 15:59:56 -0500 Subject: [PATCH 09/33] ipc: set socket receive buffer Set the sockets receive buffer size to match the send buffer. On FreeBSD without this calls to sendto() will result in an ENOBUFS error if the message is larger than net.local.dgram.recvspace sysctl. --- lib/ipc_socket.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c index e4a260688..5fc5771dc 100644 --- a/lib/ipc_socket.c +++ b/lib/ipc_socket.c @@ -112,7 +112,7 @@ set_sock_size(int sockfd, size_t max_msg_size) rc = getsockopt(sockfd, SOL_SOCKET, SO_SNDBUF, &optval, &optlen); - qb_util_log(LOG_TRACE, "%d: getsockopt(%d, needed:%d) actual:%d", + qb_util_log(LOG_TRACE, "%d: getsockopt(%d, SO_SNDBUF, needed:%d) actual:%d", rc, sockfd, max_msg_size, optval); /* The optvat <= max_msg_size check is weird... @@ -126,6 +126,25 @@ set_sock_size(int sockfd, size_t max_msg_size) optlen = sizeof(optval); rc = setsockopt(sockfd, SOL_SOCKET, SO_SNDBUF, &optval, optlen); } + + if (rc != 0) { + return rc; + } + + rc = getsockopt(sockfd, SOL_SOCKET, SO_RCVBUF, &optval, &optlen); + + qb_util_log(LOG_TRACE, "%d: getsockopt(%d, SO_RCVBUF, needed:%d) actual:%d", + rc, sockfd, max_msg_size, optval); + + /* Set the sockets receive buffer size to match the send buffer. On + * FreeBSD without this calls to sendto() will result in an ENOBUFS error + * if the message is larger than net.local.dgram.recvspace sysctl. */ + if (rc == 0 && optval <= max_msg_size) { + optval = max_msg_size; + optlen = sizeof(optval); + rc = setsockopt(sockfd, SOL_SOCKET, SO_RCVBUF, &optval, optlen); + } + return rc; } From 4978104ae490cbe388513fce0ba5d7246e6ef38b Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Sun, 14 Feb 2016 16:22:19 -0500 Subject: [PATCH 10/33] ipc: set socket buffer size used by ipcs service --- lib/ipc_socket.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c index 5fc5771dc..aeceab911 100644 --- a/lib/ipc_socket.c +++ b/lib/ipc_socket.c @@ -801,6 +801,12 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, if (res < 0) { goto cleanup_hdr; } + + res = set_sock_size(c->request.u.us.sock, c->request.max_msg_size); + if (res != 0) { + goto cleanup_hdr; + } + c->setup.u.us.sock_name = NULL; c->request.u.us.sock_name = NULL; @@ -815,6 +821,12 @@ qb_ipcs_us_connect(struct qb_ipcs_service *s, if (res < 0) { goto cleanup_hdr; } + + res = set_sock_size(c->event.u.us.sock, c->event.max_msg_size); + if (res != 0) { + goto cleanup_hdr; + } + snprintf(path, PATH_MAX, "%s-%s", r->response, "event"); c->event.u.us.sock_name = strdup(path); From 33794cce5a892af7bdab713cce89842505584ac4 Mon Sep 17 00:00:00 2001 From: David Shane Holden Date: Sun, 14 Feb 2016 16:35:43 -0500 Subject: [PATCH 11/33] ipc: return -errno when getsockopt/setsockopt fail --- lib/ipc_socket.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/ipc_socket.c b/lib/ipc_socket.c index aeceab911..68ff5e5dd 100644 --- a/lib/ipc_socket.c +++ b/lib/ipc_socket.c @@ -128,7 +128,7 @@ set_sock_size(int sockfd, size_t max_msg_size) } if (rc != 0) { - return rc; + return -errno; } rc = getsockopt(sockfd, SOL_SOCKET, SO_RCVBUF, &optval, &optlen); @@ -145,6 +145,10 @@ set_sock_size(int sockfd, size_t max_msg_size) rc = setsockopt(sockfd, SOL_SOCKET, SO_RCVBUF, &optval, optlen); } + if (rc != 0) { + return -errno; + } + return rc; } From 6e6791113f246d6d9c1cc8ddfcba816b7498b2fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Mon, 15 Feb 2016 16:52:11 +0100 Subject: [PATCH 12/33] example: simplelog supports -o (for stdout sink) --- examples/simplelog.c | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/simplelog.c b/examples/simplelog.c index 784f1af6e..eec00539e 100644 --- a/examples/simplelog.c +++ b/examples/simplelog.c @@ -73,6 +73,7 @@ show_usage(const char *name) printf("\n"); printf(" -v verbose\n"); printf(" -t threaded logging\n"); + printf(" -o log to stdout\n"); printf(" -e log to stderr\n"); printf(" -b log to blackbox\n"); printf(" -f log to a file\n"); From e74214b79b3cf18c0fcfff20d027cf6dfc9a4f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Mon, 15 Feb 2016 18:20:44 +0100 Subject: [PATCH 13/33] log: defined value as an index-pointer after static slots Amongst other uses, this allows for better (fixed) documentation regarding how many additional logging targets the user can define. --- include/qb/qblog.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/include/qb/qblog.h b/include/qb/qblog.h index 870cbae79..e7a3c202b 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -75,7 +75,8 @@ extern "C" { * * syslog, stderr and the blackbox are static (they don't need * to be created, just enabled or disabled. However you can open multiple - * logfiles (32 - QB_LOG_BLACKBOX). To do this use the following code. + * logfiles (QB_LOG_TARGET_MAX - QB_LOG_TARGET_STATIC_MAX). + * To do this, use the following code. * @code * mytarget = qb_log_file_open("/var/log/mylogfile"); * qb_log_ctl(mytarget, QB_LOG_CONF_ENABLED, QB_TRUE); @@ -397,10 +398,19 @@ void qb_log_from_external_source_va(const char *function, #define qb_enter() qb_log(LOG_TRACE, "ENTERING %s()", __func__) #define qb_leave() qb_log(LOG_TRACE, "LEAVING %s()", __func__) +/* + * Note that QB_LOG_TARGET_{STATIC_,}MAX are sentinel indexes + * as non-inclusive higher bounds of the respective categories + * (static and all the log targets) and also denote the number + * of (reserved) items in the category. Both are possibly subject + * of change, hence it is only adequate to always refer to them + * via these defined values. + */ #define QB_LOG_SYSLOG 0 #define QB_LOG_STDERR 1 #define QB_LOG_BLACKBOX 2 #define QB_LOG_STDOUT 3 +#define QB_LOG_TARGET_STATIC_MAX 4 #define QB_LOG_TARGET_MAX 32 From 4ccca4bda9e3d6cbc33451974786584d76556dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Mon, 15 Feb 2016 19:02:46 +0100 Subject: [PATCH 14/33] log: convert log target defined values into enum values Also use the new enum qb_log_target_slot type in for-loops together with a proper substitute for the literal "0" initializer. There could be more places that might be type-substituted for this enum (and hence possibly catch more of an incorrect usage if the compiler or checker has some notion of enum type narrowing), but leave it as a possible enhancement for now. --- include/qb/qblog.h | 23 +++++++++++++++++------ lib/log.c | 13 +++++++------ lib/log_format.c | 13 +++++++------ 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/include/qb/qblog.h b/include/qb/qblog.h index e7a3c202b..db729a033 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -405,14 +405,25 @@ void qb_log_from_external_source_va(const char *function, * of (reserved) items in the category. Both are possibly subject * of change, hence it is only adequate to always refer to them * via these defined values. + * Similarly, there are QB_LOG_TARGET_{STATIC_,DYNAMIC_,}START + * values, but these are inclusive lower bounds. */ -#define QB_LOG_SYSLOG 0 -#define QB_LOG_STDERR 1 -#define QB_LOG_BLACKBOX 2 -#define QB_LOG_STDOUT 3 -#define QB_LOG_TARGET_STATIC_MAX 4 +enum qb_log_target_slot { + QB_LOG_TARGET_START, -#define QB_LOG_TARGET_MAX 32 + /* static */ + QB_LOG_TARGET_STATIC_START = QB_LOG_TARGET_START, + QB_LOG_SYSLOG = QB_LOG_TARGET_STATIC_START, + QB_LOG_STDERR, + QB_LOG_BLACKBOX, + QB_LOG_STDOUT, + QB_LOG_TARGET_STATIC_MAX, + + /* dynamic */ + QB_LOG_TARGET_DYNAMIC_START = QB_LOG_TARGET_STATIC_MAX, + + QB_LOG_TARGET_MAX = 32, +}; enum qb_log_target_state { QB_LOG_STATE_UNUSED = 1, diff --git a/lib/log.c b/lib/log.c index b6eb3ce6c..da2c2615f 100644 --- a/lib/log.c +++ b/lib/log.c @@ -843,13 +843,14 @@ _log_target_state_set(struct qb_log_target *t, enum qb_log_target_state s) void qb_log_init(const char *name, int32_t facility, uint8_t priority) { - int32_t i; + int32_t l; + enum qb_log_target_slot i; - i = pthread_rwlock_init(&_listlock, NULL); - assert(i == 0); + l = pthread_rwlock_init(&_listlock, NULL); + assert(l == 0); qb_log_format_init(); - for (i = 0; i < QB_LOG_TARGET_MAX; i++) { + for (i = QB_LOG_TARGET_START; i < QB_LOG_TARGET_MAX; i++) { conf[i].pos = i; conf[i].debug = QB_FALSE; conf[i].file_sync = QB_FALSE; @@ -923,8 +924,8 @@ qb_log_fini(void) struct qb_log_target * qb_log_target_alloc(void) { - int32_t i; - for (i = 0; i < QB_LOG_TARGET_MAX; i++) { + enum qb_log_target_slot i; + for (i = QB_LOG_TARGET_START; i < QB_LOG_TARGET_MAX; i++) { if (conf[i].state == QB_LOG_STATE_UNUSED) { _log_target_state_set(&conf[i], QB_LOG_STATE_DISABLED); return &conf[i]; diff --git a/lib/log_format.c b/lib/log_format.c index 6c29f0a95..3789b6c9d 100644 --- a/lib/log_format.c +++ b/lib/log_format.c @@ -87,13 +87,14 @@ static pthread_rwlock_t _formatlock; void qb_log_format_init(void) { - int32_t i; + int32_t l; struct qb_log_target *t; + enum qb_log_target_slot i; - i = pthread_rwlock_init(&_formatlock, NULL); - assert(i == 0); + l = pthread_rwlock_init(&_formatlock, NULL); + assert(l == 0); - for (i = 0; i < QB_LOG_TARGET_MAX; i++) { + for (i = QB_LOG_TARGET_START; i < QB_LOG_TARGET_MAX; i++) { t = qb_log_target_get(i); t->format = strdup("[%p] %b"); } @@ -103,11 +104,11 @@ void qb_log_format_fini(void) { struct qb_log_target *t; - int32_t i; + enum qb_log_target_slot i; pthread_rwlock_destroy(&_formatlock); - for (i = 0; i < QB_LOG_TARGET_MAX; i++) { + for (i = QB_LOG_TARGET_START; i < QB_LOG_TARGET_MAX; i++) { t = qb_log_target_get(i); free(t->format); } From 3ef3ae434faeb4ee11889bfcefc7cfe5b0fa949f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 16 Feb 2016 17:44:03 +0100 Subject: [PATCH 15/33] doc: main overview: fix style + reword per spec file --- docs/mainpage.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/docs/mainpage.h b/docs/mainpage.h index 6a6761de0..5e1f1e7da 100644 --- a/docs/mainpage.h +++ b/docs/mainpage.h @@ -3,15 +3,13 @@ * * @section overview Overview * - * libqb is a thread-safe library with the primary purpose of providing high - * performance client server reusable features. + * libqb is a thread-safe library with the primary purpose of providing + * high-performance, reusable features for client-server architecture, + * such as logging, tracing, inter-process communication (IPC), and polling. * - * It provides high performance ipc, and poll. - * - * We don't intend be an all encompassing library, but instead provide very - * specially focused APIs that are highly - * - * tuned for maximum performance for client/server applications. + * We don't intend this to be an all-encompassing library, but instead + * provide very specially focused APIs that are highly tuned for maximum + * performance for client/server applications. * * See the following pages for more info: * - @subpage qb_list_overview From fdbb80a87b2f49b5f2776a1b7198442e9a27b256 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 16 Feb 2016 17:47:05 +0100 Subject: [PATCH 16/33] doc: IPC overview: fix typos --- docs/mainpage.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/mainpage.h b/docs/mainpage.h index 5e1f1e7da..aff769e6c 100644 --- a/docs/mainpage.h +++ b/docs/mainpage.h @@ -70,7 +70,7 @@ * @page qb_ipc_overview IPC Overview * * @par Overview - * libqb provides a generically reusable very high performance shared memory IPC sytem for client + * libqb provides a generically reusable very high performance shared memory IPC system for client * and service applications. It supports many features including: * - Multiple transport implementations * -# Shared memory implementation for very high performance. @@ -89,7 +89,7 @@ * then determines if the UID and GID are authenticated for communication. * * @par Performance - * For performance QB_IPC_SHM (shared memory) is recogmended. It is tuned for + * For performance, QB_IPC_SHM (shared memory) is recommended. It is tuned for * very high performance. * * @par Client API From 807c7dc34ac55ae0d43198b1f825556a5183901f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Mon, 15 Feb 2016 19:57:43 +0100 Subject: [PATCH 17/33] log: better (fixed) documentation for funcs returning "slot" index Also add missing "stdout" to the enumerations of the static ones. --- include/qb/qblog.h | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/include/qb/qblog.h b/include/qb/qblog.h index db729a033..4d6099ca1 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -65,7 +65,7 @@ extern "C" { * @endcode * * @par Configuring log targets. - * A log target can by syslog, stderr, the blackbox or a text file. + * A log target can by syslog, stderr, the blackbox, stdout, or a text file. * By default only syslog is enabled. * * To enable a target do the following @@ -73,7 +73,7 @@ extern "C" { * qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); * @endcode * - * syslog, stderr and the blackbox are static (they don't need + * syslog, stderr, the blackbox, and stdout are static (they don't need * to be created, just enabled or disabled. However you can open multiple * logfiles (QB_LOG_TARGET_MAX - QB_LOG_TARGET_STATIC_MAX). * To do this, use the following code. @@ -406,7 +406,8 @@ void qb_log_from_external_source_va(const char *function, * of change, hence it is only adequate to always refer to them * via these defined values. * Similarly, there are QB_LOG_TARGET_{STATIC_,DYNAMIC_,}START - * values, but these are inclusive lower bounds. + * and QB_LOG_TARGET_{STATIC_,DYNAMIC_,}END values, but these + * are inclusive lower and higher bounds, respectively. */ enum qb_log_target_slot { QB_LOG_TARGET_START, @@ -418,11 +419,14 @@ enum qb_log_target_slot { QB_LOG_BLACKBOX, QB_LOG_STDOUT, QB_LOG_TARGET_STATIC_MAX, + QB_LOG_TARGET_STATIC_END = QB_LOG_TARGET_STATIC_MAX - 1, /* dynamic */ QB_LOG_TARGET_DYNAMIC_START = QB_LOG_TARGET_STATIC_MAX, QB_LOG_TARGET_MAX = 32, + QB_LOG_TARGET_DYNAMIC_END = QB_LOG_TARGET_MAX - 1, + QB_LOG_TARGET_END = QB_LOG_TARGET_DYNAMIC_END, }; enum qb_log_target_state { @@ -589,7 +593,9 @@ void qb_log_format_set(int32_t t, const char* format); * Open a log file. * * @retval -errno on error - * @retval 3 to 31 (to be passed into other qb_log_* functions) + * @retval value in inclusive range QB_LOG_TARGET_DYNAMIC_START + * to QB_LOG_TARGET_DYNAMIC_END + * (to be passed into other qb_log_* functions) */ int32_t qb_log_file_open(const char *filename); @@ -625,7 +631,9 @@ void qb_log_blackbox_print_from_file(const char* filename); * Open a custom log target. * * @retval -errno on error - * @retval 3 to 31 (to be passed into other qb_log_* functions) + * @retval value in inclusive range QB_LOG_TARGET_DYNAMIC_START + * to QB_LOG_TARGET_DYNAMIC_END + * (to be passed into other qb_log_* functions) */ int32_t qb_log_custom_open(qb_log_logger_fn log_fn, qb_log_close_fn close_fn, From 2198893c3ea0029d77b24a4be24f1be11a4bae8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 16 Feb 2016 10:37:58 +0100 Subject: [PATCH 18/33] log: refactor static target slots state initialization It doesn't matter that also syslog is disabled first, as it will get enabled few lines later on (uniformity + simplicity > optimization + complexity). --- lib/log.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/log.c b/lib/log.c index da2c2615f..52c906ece 100644 --- a/lib/log.c +++ b/lib/log.c @@ -868,9 +868,8 @@ qb_log_init(const char *name, int32_t facility, uint8_t priority) _log_so_walk_dlnames(); #endif /* QB_HAVE_ATTRIBUTE_SECTION */ - conf[QB_LOG_STDERR].state = QB_LOG_STATE_DISABLED; - conf[QB_LOG_BLACKBOX].state = QB_LOG_STATE_DISABLED; - conf[QB_LOG_STDOUT].state = QB_LOG_STATE_DISABLED; + for (i = QB_LOG_TARGET_STATIC_START; i < QB_LOG_TARGET_STATIC_MAX; i++) + conf[i].state = QB_LOG_STATE_DISABLED; logger_inited = QB_TRUE; (void)qb_log_syslog_open(&conf[QB_LOG_SYSLOG]); From 7a290093d003aed26f7129de1b0042facbb5ce2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 16 Feb 2016 10:42:03 +0100 Subject: [PATCH 19/33] log: convert few more instances to use enum qb_log_target_slot --- lib/log.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/log.c b/lib/log.c index 52c906ece..6a178361d 100644 --- a/lib/log.c +++ b/lib/log.c @@ -204,7 +204,7 @@ qb_log_real_va_(struct qb_log_callsite *cs, va_list ap) int32_t found_threaded = QB_FALSE; struct qb_log_target *t; struct timespec tv; - int32_t pos; + enum qb_log_target_slot pos; int32_t formatted = QB_FALSE; char buf[QB_LOG_MAX_LEN]; char *str = buf; @@ -232,7 +232,7 @@ qb_log_real_va_(struct qb_log_callsite *cs, va_list ap) * 1 if we can find a threaded target that needs this log then post it * 2 foreach non-threaded target call it's logger function */ - for (pos = 0; pos <= conf_active_max; pos++) { + for (pos = QB_LOG_TARGET_START; pos <= conf_active_max; pos++) { t = &conf[pos]; if ((t->state == QB_LOG_STATE_ENABLED) && qb_bit_is_set(cs->targets, pos)) { @@ -282,9 +282,9 @@ qb_log_thread_log_write(struct qb_log_callsite *cs, time_t timestamp, const char *buffer) { struct qb_log_target *t; - int32_t pos; + enum qb_log_target_slot pos; - for (pos = 0; pos <= conf_active_max; pos++) { + for (pos = QB_LOG_TARGET_START; pos <= conf_active_max; pos++) { t = &conf[pos]; if ((t->state == QB_LOG_STATE_ENABLED) && t->threaded && qb_bit_is_set(cs->targets, t->pos)) { @@ -307,7 +307,7 @@ qb_log_callsite_get(const char *function, struct qb_log_callsite *cs; int32_t new_dcs = QB_FALSE; struct qb_list_head *f_item; - int32_t pos; + enum qb_log_target_slot pos; if (!logger_inited) { return NULL; @@ -321,7 +321,7 @@ qb_log_callsite_get(const char *function, if (new_dcs) { pthread_rwlock_rdlock(&_listlock); - for (pos = 0; pos <= conf_active_max; pos++) { + for (pos = QB_LOG_TARGET_START; pos <= conf_active_max; pos++) { t = &conf[pos]; if (t->state != QB_LOG_STATE_ENABLED) { continue; @@ -403,7 +403,7 @@ qb_log_callsites_register(struct qb_log_callsite *_start, struct qb_log_callsite *cs; struct qb_log_target *t; struct qb_log_filter *flt; - int32_t pos; + enum qb_log_target_slot pos; if (_start == NULL || _stop == NULL) { return -EINVAL; @@ -432,7 +432,7 @@ qb_log_callsites_register(struct qb_log_callsite *_start, /* * Now apply the filters on these new callsites */ - for (pos = 0; pos <= conf_active_max; pos++) { + for (pos = QB_LOG_TARGET_START; pos <= conf_active_max; pos++) { t = &conf[pos]; if (t->state != QB_LOG_STATE_ENABLED) { continue; @@ -823,18 +823,18 @@ _log_so_walk_dlnames(void) static void _log_target_state_set(struct qb_log_target *t, enum qb_log_target_state s) { - int32_t i; + enum qb_log_target_slot i; int32_t a_set = QB_FALSE; int32_t u_set = QB_FALSE; t->state = s; - for (i = 31; i >= 0; i--) { - if (!a_set && conf[i].state == QB_LOG_STATE_ENABLED) { + for (i = QB_LOG_TARGET_MAX; i > QB_LOG_TARGET_START; i--) { + if (!a_set && conf[i-1].state == QB_LOG_STATE_ENABLED) { a_set = QB_TRUE; - conf_active_max = i; + conf_active_max = i-1; } - if (!u_set && conf[i].state != QB_LOG_STATE_UNUSED) { + if (!u_set && conf[i-1].state != QB_LOG_STATE_UNUSED) { u_set = QB_TRUE; } } @@ -888,7 +888,7 @@ qb_log_fini(void) struct qb_list_head *iter2; struct qb_list_head *next; struct qb_list_head *next2; - int32_t pos; + enum qb_log_target_slot pos; if (!logger_inited) { return; @@ -897,7 +897,7 @@ qb_log_fini(void) qb_log_thread_stop(); pthread_rwlock_destroy(&_listlock); - for (pos = 0; pos <= conf_active_max; pos++) { + for (pos = QB_LOG_TARGET_START; pos <= conf_active_max; pos++) { t = &conf[pos]; _log_target_disable(t); qb_list_for_each_safe(iter2, next2, &t->filter_head) { From 78d5d1686ada4fd9c28fd3a31d0bb93a66677a3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 16 Feb 2016 14:40:25 +0100 Subject: [PATCH 20/33] qblog.h: unify descriptions before the code examples - use semicolons at the end of the description - use commas when there can be an issue with parsing a long sentence (and wrap such text at 79 characters if applicable) --- include/qb/qblog.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/include/qb/qblog.h b/include/qb/qblog.h index 4d6099ca1..b28a04682 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -68,7 +68,7 @@ extern "C" { * A log target can by syslog, stderr, the blackbox, stdout, or a text file. * By default only syslog is enabled. * - * To enable a target do the following + * To enable a target do the following: * @code * qb_log_ctl(QB_LOG_BLACKBOX, QB_LOG_CONF_ENABLED, QB_TRUE); * @endcode @@ -76,7 +76,7 @@ extern "C" { * syslog, stderr, the blackbox, and stdout are static (they don't need * to be created, just enabled or disabled. However you can open multiple * logfiles (QB_LOG_TARGET_MAX - QB_LOG_TARGET_STATIC_MAX). - * To do this, use the following code. + * To do this, use the following code: * @code * mytarget = qb_log_file_open("/var/log/mylogfile"); * qb_log_ctl(mytarget, QB_LOG_CONF_ENABLED, QB_TRUE); @@ -115,19 +115,21 @@ extern "C" { * -# function name + priority * -# format string + priority * - * So to make all logs from evil_fnunction() go to stderr do the following: + * So to make all logs from evil_fnunction() go to stderr, do the following: * @code * qb_log_filter_ctl(QB_LOG_STDERR, QB_LOG_FILTER_ADD, * QB_LOG_FILTER_FUNCTION, "evil_fnunction", LOG_TRACE); * @endcode * - * So to make all logs from totem* (with a priority <= LOG_INFO) go to stderr do the following: + * So to make all logs from totem* (with a priority <= LOG_INFO) go to stderr, + * do the following: * @code * qb_log_filter_ctl(QB_LOG_STDERR, QB_LOG_FILTER_ADD, * QB_LOG_FILTER_FILE, "totem", LOG_INFO); * @endcode * - * So to make all logs with the substring "ringbuffer" go to stderr do the following: + * So to make all logs with the substring "ringbuffer" go to stderr, + * do the following: * @code * qb_log_filter_ctl(QB_LOG_STDERR, QB_LOG_FILTER_ADD, * QB_LOG_FILTER_FORMAT, "ringbuffer", LOG_TRACE); @@ -501,7 +503,7 @@ void qb_log_fini(void); * If you are using dynamically loadable modules via dlopen() and * you load them after qb_log_init() then after you load the module * you will need to do the following to get the filters to work - * in that module. + * in that module: * @code * _start = dlsym (dl_handle, "__start___verbose"); * _stop = dlsym (dl_handle, "__stop___verbose"); @@ -549,7 +551,7 @@ int32_t qb_log_filter_ctl2(int32_t value, enum qb_log_filter_conf c, * Instead of using the qb_log_filter_ctl() functions you * can apply the filters manually by defining a callback * and setting the targets field using qb_bit_set() and - * qb_bit_clear() like the following below. + * qb_bit_clear() like the following below: * @code * static void * m_filter(struct qb_log_callsite *cs) From 837108b02aa201e8a1519a291c361a08817e4775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Tue, 16 Feb 2016 14:43:00 +0100 Subject: [PATCH 21/33] qblog.h: fix typos and make stylistic enhancements --- include/qb/qblog.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/include/qb/qblog.h b/include/qb/qblog.h index b28a04682..8ce16c93c 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -53,7 +53,7 @@ extern "C" { * Call qb_log() to generate a log message. Then to write the message * somewhere meaningful call qb_log_ctl() to configure the targets. * - * Simplist possible use: + * Simplest possible use: * @code * main() { * qb_log_init("simple-log", LOG_DAEMON, LOG_INFO); @@ -65,7 +65,7 @@ extern "C" { * @endcode * * @par Configuring log targets. - * A log target can by syslog, stderr, the blackbox, stdout, or a text file. + * A log target can be syslog, stderr, the blackbox, stdout, or a text file. * By default only syslog is enabled. * * To enable a target do the following: @@ -74,7 +74,7 @@ extern "C" { * @endcode * * syslog, stderr, the blackbox, and stdout are static (they don't need - * to be created, just enabled or disabled. However you can open multiple + * to be created, just enabled or disabled). However you can open multiple * logfiles (QB_LOG_TARGET_MAX - QB_LOG_TARGET_STATIC_MAX). * To do this, use the following code: * @code @@ -115,10 +115,10 @@ extern "C" { * -# function name + priority * -# format string + priority * - * So to make all logs from evil_fnunction() go to stderr, do the following: + * So to make all logs from evil_function() go to stderr, do the following: * @code * qb_log_filter_ctl(QB_LOG_STDERR, QB_LOG_FILTER_ADD, - * QB_LOG_FILTER_FUNCTION, "evil_fnunction", LOG_TRACE); + * QB_LOG_FILTER_FUNCTION, "evil_function", LOG_TRACE); * @endcode * * So to make all logs from totem* (with a priority <= LOG_INFO) go to stderr, @@ -141,8 +141,8 @@ extern "C" { * and use qg_log_filter_ctl to set the QB_LOG_CONF_THREADED flag on all the * logging targets in use. * - * To achieve non-blocking logging you can use threaded logging as well - * So any calls to write() or syslog() will not hold up your program. + * To achieve non-blocking logging, so that any calls to write() or syslog() + * will not hold up your program, you can use threaded logging as well. * * Threaded logging use: * @code @@ -281,7 +281,7 @@ void qb_log_real_va_(struct qb_log_callsite *cs, va_list ap); * * @note the performance of this will not impress you, as * the filtering is done on each log message, not - * before hand. So try doing basic pre-filtering. + * beforehand. So try doing basic pre-filtering. * * @param function originating function name * @param filename originating filename @@ -303,7 +303,7 @@ void qb_log_from_external_source(const char *function, __attribute__ ((format (printf, 3, 7))); /** - * Get or create a callsite at the give position. + * Get or create a callsite at the given position. * * The result can then be passed into qb_log_real_() * @@ -495,7 +495,7 @@ void qb_log_init(const char *name, * * It releases any shared memory. * Stops the logging thread if running. - * Flushes the last message to their destinations. + * Flushes the last messages to their destinations. */ void qb_log_fini(void); From 642f74de61987ddf7672ddfcff1303f4b70ce47b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Thu, 11 Feb 2016 20:42:35 +0100 Subject: [PATCH 22/33] Feature: allow changing the identifier for syslog (+tests) Original "qb_log_ctl" interface had to be extended for passing read-only strings (new parameter), resulting in new "qb_log_ctl2" function, which is what qb_log_ctl calls into with the new parameter set to NULL. This ensures backward compatibility. A new QB_LOG_CONF_IDENT configuration directive for the mentioned interface is added with a goal to set new internal identifier that is, notably, used for syslog sink. This allows for switching the identification without a need to reinitialize logging subsystem, akin to changing target logging facility. Also a brand new concept of testing syslog sink in particular is introduced (finally). During initial trial&error stage, it used LD_PRELOAD hack but it seems that libtool is sophisticated enough that no such extra intervention is needed and the desired symbol resolution Just Works (tm). However, the technique is highly non-portable (there is even a warning about that from libtool, which is partially on purpose as the _syslog_override.so should rather be explicit it is by no mean a regular library) and hence the syslog tests have to be enabled with explicit ./configure --enable-syslog-tests rather than possibly break on untested platforms (far too many). The concept can be extended upon, but initially, just the new feature is being tested. Post-review: thanks Chrissie for a suggestion how to deal with extract-arg-and-forget in a less intrusive way (no defines). --- configure.ac | 12 ++++++++ include/qb/qblog.h | 25 ++++++++++++++++ lib/log.c | 34 ++++++++++++++++------ tests/Makefile.am | 7 +++++ tests/_syslog_override.c | 62 ++++++++++++++++++++++++++++++++++++++++ tests/_syslog_override.h | 34 ++++++++++++++++++++++ tests/check_log.c | 29 +++++++++++++++++++ 7 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 tests/_syslog_override.c create mode 100644 tests/_syslog_override.h diff --git a/configure.ac b/configure.ac index 50e154941..3cd47ee58 100644 --- a/configure.ac +++ b/configure.ac @@ -434,6 +434,12 @@ AC_ARG_ENABLE([slow-tests], [ --enable-slow-tests : build and run slow tests. ], [ default="no" ]) +if test x"$with_check" = xyes; then +AC_ARG_ENABLE([syslog-tests], + [ --enable-syslog-tests : build and run syslog tests. ], + [ default="no" ]) +fi + AC_ARG_WITH([socket-dir], [ --with-socket-dir=DIR : socket dir. ], [ SOCKETDIR="$withval" ], @@ -514,6 +520,12 @@ if test "x${enable_slow_tests}" = xyes ; then fi AM_CONDITIONAL(HAVE_SLOW_TESTS, [test "x${enable_slow_tests}" = xyes]) AC_SUBST(HAVE_SLOW_TESTS) +if test "x${enable_syslog_tests}" = xyes ; then + AC_DEFINE([HAVE_SYSLOG_TESTS], 1,[have syslog tests]) + AC_MSG_NOTICE([Enabling syslog tests]) +fi +AM_CONDITIONAL(HAVE_SYSLOG_TESTS, [test "x${enable_syslog_tests}" = xyes]) +AC_SUBST(HAVE_SYSLOG_TESTS) # --- callsite sections --- if test "x${GCC}" = xyes; then diff --git a/include/qb/qblog.h b/include/qb/qblog.h index 870cbae79..0abb90eb5 100644 --- a/include/qb/qblog.h +++ b/include/qb/qblog.h @@ -420,6 +420,7 @@ enum qb_log_conf { QB_LOG_CONF_STATE_GET, QB_LOG_CONF_FILE_SYNC, QB_LOG_CONF_EXTENDED, + QB_LOG_CONF_IDENT, }; enum qb_log_filter_type { @@ -504,6 +505,30 @@ void qb_log_callsites_dump(void); */ int32_t qb_log_ctl(int32_t target, enum qb_log_conf conf_type, int32_t arg); +typedef union { + int32_t i32; + const char *s; +} qb_log_ctl2_arg_t; + +/** + * Extension of main logging control function accepting also strings. + * + * @param arg for QB_LOG_CONF_IDENT, 's' member as new identifier to openlog(), + * for all original qb_log_ctl-compatible configuration directives, + * 'i32' member is assumed (although a preferred way is to use + * that original function directly as it allows for more type safety) + * @see qb_log_ctl + * + * @note You can use QB_LOG_CTL2_I32 and QB_LOG_CTL2_S + * macros for a convenient on-the-fly construction of the object + * to be passed as an arg argument. + */ +int32_t qb_log_ctl2(int32_t target, enum qb_log_conf conf_type, + qb_log_ctl2_arg_t arg); + +#define QB_LOG_CTL2_I32(a) ((qb_log_ctl2_arg_t) { .i32 = (a) }) +#define QB_LOG_CTL2_S(a) ((qb_log_ctl2_arg_t) { .s = (a) }) + /** * This allows you modify the 'tags' and 'targets' callsite fields at runtime. */ diff --git a/lib/log.c b/lib/log.c index b6eb3ce6c..e851d8900 100644 --- a/lib/log.c +++ b/lib/log.c @@ -1060,11 +1060,15 @@ _log_target_disable(struct qb_log_target *t) } int32_t -qb_log_ctl(int32_t t, enum qb_log_conf c, int32_t arg) +qb_log_ctl2(int32_t t, enum qb_log_conf c, qb_log_ctl2_arg_t arg_not4directuse) { int32_t rc = 0; int32_t need_reload = QB_FALSE; + /* extract the constants and do not touch the origin anymore */ + const int32_t arg_i32 = arg_not4directuse.i32; + const char * const arg_s = arg_not4directuse.s; + if (!logger_inited) { return -EINVAL; } @@ -1074,7 +1078,7 @@ qb_log_ctl(int32_t t, enum qb_log_conf c, int32_t arg) } switch (c) { case QB_LOG_CONF_ENABLED: - if (arg) { + if (arg_i32) { rc = _log_target_enable(&conf[t]); } else { _log_target_disable(&conf[t]); @@ -1084,33 +1088,39 @@ qb_log_ctl(int32_t t, enum qb_log_conf c, int32_t arg) rc = conf[t].state; break; case QB_LOG_CONF_FACILITY: - conf[t].facility = arg; + conf[t].facility = arg_i32; + if (t == QB_LOG_SYSLOG) { + need_reload = QB_TRUE; + } + break; + case QB_LOG_CONF_IDENT: + (void)strlcpy(conf[t].name, arg_s, PATH_MAX); if (t == QB_LOG_SYSLOG) { need_reload = QB_TRUE; } break; case QB_LOG_CONF_FILE_SYNC: - conf[t].file_sync = arg; + conf[t].file_sync = arg_i32; break; case QB_LOG_CONF_PRIORITY_BUMP: - conf[t].priority_bump = arg; + conf[t].priority_bump = arg_i32; break; case QB_LOG_CONF_SIZE: if (t == QB_LOG_BLACKBOX) { - if (arg <= 0) { + if (arg_i32 <= 0) { return -EINVAL; } - conf[t].size = arg; + conf[t].size = arg_i32; need_reload = QB_TRUE; } else { return -ENOSYS; } break; case QB_LOG_CONF_THREADED: - conf[t].threaded = arg; + conf[t].threaded = arg_i32; break; case QB_LOG_CONF_EXTENDED: - conf[t].extended = arg; + conf[t].extended = arg_i32; break; default: @@ -1123,3 +1133,9 @@ qb_log_ctl(int32_t t, enum qb_log_conf c, int32_t arg) } return rc; } + +int32_t +qb_log_ctl(int32_t t, enum qb_log_conf c, int32_t arg) +{ + return qb_log_ctl2(t, c, QB_LOG_CTL2_I32(arg)); +} diff --git a/tests/Makefile.am b/tests/Makefile.am index aaeb41582..807ae2e2d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -147,6 +147,13 @@ ipc_test_LDADD = $(top_builddir)/lib/libqb.la @CHECK_LIBS@ log_test_SOURCES = check_log.c $(top_builddir)/include/qb/qblog.h log_test_CFLAGS = @CHECK_CFLAGS@ -I$(top_srcdir)/include log_test_LDADD = $(top_builddir)/lib/libqb.la @CHECK_LIBS@ +if HAVE_SYSLOG_TESTS +log_test_LDADD += _syslog_override.la + +lib_LTLIBRARIES = _syslog_override.la +_syslog_override_la_SOURCES = _syslog_override.c +_syslog_override_la_LDFLAGS = -module +endif util_test_SOURCES = check_util.c $(top_builddir)/include/qb/qbutil.h util_test_CFLAGS = @CHECK_CFLAGS@ -I$(top_srcdir)/include diff --git a/tests/_syslog_override.c b/tests/_syslog_override.c new file mode 100644 index 000000000..0c3a809d7 --- /dev/null +++ b/tests/_syslog_override.c @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2016 Red Hat, Inc. + * + * All rights reserved. + * + * Author: Jan Pokorny + * + * This file is part of libqb. + * + * libqb is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 2.1 of the License, or + * (at your option) any later version. + * + * libqb is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with libqb. If not, see . + */ + +#include "_syslog_override.h" + +#include +#include + +int _syslog_opened = 0; +int _syslog_option = 0; +int _syslog_facility = 0; +char _syslog_ident[PATH_MAX] = ""; + +void openlog(const char *ident, int option, int facility); + +void +openlog(const char *ident, int option, int facility) +{ + _syslog_opened = 1; + _syslog_option = option; + _syslog_facility = facility; + strncpy(_syslog_ident, ident, sizeof(_syslog_ident)); +} + +void syslog(int priority, const char *format, ...); + +void +syslog(int priority, const char *format, ...) +{ + _syslog_opened = 1; +} + +void closelog(void); + +void +closelog(void) +{ + _syslog_opened = 0; + _syslog_option = -1; + _syslog_facility = -1; + _syslog_ident[0] = '\0'; +} diff --git a/tests/_syslog_override.h b/tests/_syslog_override.h new file mode 100644 index 000000000..7b254aea0 --- /dev/null +++ b/tests/_syslog_override.h @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2016 Red Hat, Inc. + * + * All rights reserved. + * + * Author: Jan Pokorny + * + * This file is part of libqb. + * + * libqb is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 2.1 of the License, or + * (at your option) any later version. + * + * libqb is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with libqb. If not, see . + */ + +#ifndef QB_SYSLOG_OVERRIDE_H_DEFINED +#define QB_SYSLOG_OVERRIDE_H_DEFINED + +#include + +extern int _syslog_opened; +extern int _syslog_option; +extern int _syslog_facility; +extern char _syslog_ident[PATH_MAX]; + +#endif diff --git a/tests/check_log.c b/tests/check_log.c index 56c9305f1..d7ac70bc8 100644 --- a/tests/check_log.c +++ b/tests/check_log.c @@ -29,6 +29,10 @@ #include #include +#ifdef HAVE_SYSLOG_TESTS +#include "_syslog_override.h" +#endif + extern size_t qb_vsnprintf_serialize(char *serialize, size_t max_len, const char *fmt, va_list ap); extern size_t qb_vsnprintf_deserialize(char *string, size_t strlen, const char *buf); @@ -765,6 +769,25 @@ START_TEST(test_extended_information) } END_TEST +#ifdef HAVE_SYSLOG_TESTS +START_TEST(test_syslog) +{ + qb_log_init("flip", LOG_USER, LOG_INFO); + qb_log_ctl(QB_LOG_SYSLOG, QB_LOG_CONF_ENABLED, QB_TRUE); + + qb_log(LOG_ERR, "first as flip"); + ck_assert_int_eq(_syslog_opened, 1); + ck_assert_str_eq(_syslog_ident, "flip"); + + qb_log_ctl2(QB_LOG_SYSLOG, QB_LOG_CONF_IDENT, QB_LOG_CTL2_S("flop")); + qb_log(LOG_ERR, "second as flop"); + ck_assert_str_eq(_syslog_ident, "flop"); + + qb_log_fini(); +} +END_TEST +#endif + static Suite * log_suite(void) { @@ -812,6 +835,12 @@ log_suite(void) tcase_add_test(tc, test_extended_information); suite_add_tcase(s, tc); +#ifdef HAVE_SYSLOG_TESTS + tc = tcase_create("syslog"); + tcase_add_test(tc, test_syslog); + suite_add_tcase(s, tc); +#endif + return s; } From 0fc27575464afb585a7f0a569f6ec48c24a196d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Fri, 12 Feb 2016 01:27:27 +0100 Subject: [PATCH 23/33] CI: .travis.yml cosmetics --- .travis.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index eef8bf9bb..297238b0c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,12 @@ language: c -compiler: - - gcc +compiler: gcc addons: apt: packages: - check - splint -script: ./autogen.sh && ./configure && make check && make distcheck +script: ./autogen.sh + && ./configure + && make check + && make distcheck sudo: false From fa73c21ae4a743fc8c45157e30642384236fded2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Fri, 12 Feb 2016 01:29:29 +0100 Subject: [PATCH 24/33] CI: enable recently added syslog-tests in .travis.yml --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 297238b0c..1ed9801e2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ addons: - check - splint script: ./autogen.sh - && ./configure + && ./configure --enable-syslog-tests && make check && make distcheck sudo: false From 7b10be740197f6f027e440dfd54394044708f590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Thu, 18 Feb 2016 11:47:34 +0100 Subject: [PATCH 25/33] CI: restore travis notifications, IRC this time around While quarterback [dash] devel [at] lists [dot] fedorahosted [dot] org, original notification target and development ML seems long gone (without archives preserved!), let's use #clusterlabs-dev at freenode IRC network. This is in-line with the purpose of the channel and it's what clufter, crmsh and hawk seem to be currently using. --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 1ed9801e2..6b9a01b00 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,3 +10,6 @@ script: ./autogen.sh && make check && make distcheck sudo: false + +notifications: + irc: "irc.freenode.net#clusterlabs-dev" From c36a00929b4eb1709debfe1c793c21c31e03e556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Thu, 18 Feb 2016 16:39:14 +0100 Subject: [PATCH 26/33] Low: no magic constants + gethostname usage sanity --- lib/log_format.c | 7 ++++--- tests/check_log.c | 13 +++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/log_format.c b/lib/log_format.c index 6c29f0a95..82fa1eb03 100644 --- a/lib/log_format.c +++ b/lib/log_format.c @@ -272,10 +272,11 @@ qb_log_target_format_static(int32_t target, const char * format, break; case 'H': - if (gethostname(tmp_buf, 255) == 0) { - tmp_buf[254] = '\0'; + if (gethostname(tmp_buf, sizeof(tmp_buf)) == 0) { + tmp_buf[sizeof(tmp_buf) - 1] = '\0'; } else { - (void)strlcpy(tmp_buf, "localhost", 255); + (void)strlcpy(tmp_buf, "localhost", + sizeof(tmp_buf)); } p = tmp_buf; break; diff --git a/tests/check_log.c b/tests/check_log.c index d7ac70bc8..853862016 100644 --- a/tests/check_log.c +++ b/tests/check_log.c @@ -450,8 +450,10 @@ static const char *_test_tags_stringify(uint32_t tags) START_TEST(test_log_format) { int32_t t; - char cmp_str[256]; + /* following size/length related equation holds in the context of use: + strlen(cmp_str) = strlen(host_str) + X; X ~ 20 < sizeof(host_str) */ char host_str[256]; + char cmp_str[2 * sizeof(host_str)]; qb_log_init("test", LOG_USER, LOG_DEBUG); qb_log_ctl(QB_LOG_SYSLOG, QB_LOG_CONF_ENABLED, QB_FALSE); @@ -498,16 +500,19 @@ START_TEST(test_log_format) qb_log_tags_stringify_fn_set(NULL); - gethostname(host_str, 256); + gethostname(host_str, sizeof(host_str)); + host_str[sizeof(host_str) - 1] = '\0'; qb_log_format_set(t, "%P %H %N %b"); qb_log(LOG_INFO, "Angus"); - snprintf(cmp_str, 256, "%d %s test Angus", getpid(), host_str); + snprintf(cmp_str, sizeof(cmp_str), "%d %s test Angus", getpid(), + host_str); ck_assert_str_eq(test_buf, cmp_str); qb_log_format_set(t, "%3N %H %P %b"); qb_log(LOG_INFO, "Angus"); - snprintf(cmp_str, 256, "tes %s %d Angus", host_str, getpid()); + snprintf(cmp_str, sizeof(cmp_str), "tes %s %d Angus", host_str, + getpid()); ck_assert_str_eq(test_buf, cmp_str); } END_TEST From cabe021d47bd16431a3889471066c6fc3ef1e1ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Thu, 18 Feb 2016 18:28:03 +0100 Subject: [PATCH 27/33] build: fix "dependant" typo --- docs/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 0c6eaa3d2..61ddc0f07 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -24,14 +24,14 @@ noinst_HEADERS = mainpage.h dist_man_MANS = man8/qb-blackbox.8 if HAVE_DOXYGEN inc_dir = $(top_srcdir)/include/qb -dependant_headers = $(wildcard $(inc_dir)/qb*.h) +dependent_headers = $(wildcard $(inc_dir)/qb*.h) dist_man_MANS += man3/qbipcc.h.3 man3/qbipcs.h.3 man3/qbatomic.h.3 \ man3/qbhdb.h.3 man3/qbipc_common.h.3 man3/qblist.h.3 \ man3/qbloop.h.3 man3/qbutil.h.3 man3/qbarray.h.3 \ man3/qblog.h.3 man3/qbmap.h.3 -$(dist_man_MANS): man.dox $(dependant_headers) +$(dist_man_MANS): man.dox $(dependent_headers) mkdir -p man3 doxygen man.dox From 0b04ed5e77bddd115ec86a6421f6171f19ed4250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Thu, 18 Feb 2016 18:32:08 +0100 Subject: [PATCH 28/33] build: grab "dependent_headers" from respective Makefile.am ...with "echo path/*.h" fallback (equivalent of wildcard function) if the new methods fails for some reason. --- docs/Makefile.am | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 61ddc0f07..957a9317e 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -24,14 +24,19 @@ noinst_HEADERS = mainpage.h dist_man_MANS = man8/qb-blackbox.8 if HAVE_DOXYGEN inc_dir = $(top_srcdir)/include/qb -dependent_headers = $(wildcard $(inc_dir)/qb*.h) + +dependent_headers = $(subst $(inc_dir),,$(shell \ + printf 'include $(inc_dir)/Makefile.am\n\n%%.var:\n\t@echo $$($$*)' \ + | ${MAKE} --no-print-directory -f - inst_HEADERS.var \ + || echo $(inc_dir)/qb*.h)) + dist_man_MANS += man3/qbipcc.h.3 man3/qbipcs.h.3 man3/qbatomic.h.3 \ man3/qbhdb.h.3 man3/qbipc_common.h.3 man3/qblist.h.3 \ man3/qbloop.h.3 man3/qbutil.h.3 man3/qbarray.h.3 \ man3/qblog.h.3 man3/qbmap.h.3 -$(dist_man_MANS): man.dox $(dependent_headers) +$(dist_man_MANS): man.dox $(dependent_headers:%=$(inc_dir)/%) mkdir -p man3 doxygen man.dox From cf1588c6ecb113db922112837163fd3a9668cdfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Thu, 18 Feb 2016 18:56:54 +0100 Subject: [PATCH 29/33] build: header-based man pages: dependent_headers - blacklist IOW, make tracking of interfaces to document via man pages maintainable. --- docs/Makefile.am | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 957a9317e..32c6d08f9 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -29,12 +29,11 @@ dependent_headers = $(subst $(inc_dir),,$(shell \ printf 'include $(inc_dir)/Makefile.am\n\n%%.var:\n\t@echo $$($$*)' \ | ${MAKE} --no-print-directory -f - inst_HEADERS.var \ || echo $(inc_dir)/qb*.h)) +dependent_headers_omit = qbconfig.h qbdefs.h qbrb.h -dist_man_MANS += man3/qbipcc.h.3 man3/qbipcs.h.3 man3/qbatomic.h.3 \ - man3/qbhdb.h.3 man3/qbipc_common.h.3 man3/qblist.h.3 \ - man3/qbloop.h.3 man3/qbutil.h.3 man3/qbarray.h.3 \ - man3/qblog.h.3 man3/qbmap.h.3 - +dist_man_MANS += $(patsubst %,man3/%.3,$(filter-out \ + $(dependent_headers_omit),$(dependent_headers) \ + )) $(dist_man_MANS): man.dox $(dependent_headers:%=$(inc_dir)/%) mkdir -p man3 From 792c34bf6b8b25af0e675e7e3d2d02dc1428715a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Thu, 18 Feb 2016 19:04:48 +0100 Subject: [PATCH 30/33] build: header-based man pages: include also qbdefs.h+qbrb.h No reason not to do that. Situation with qbconfig.h is a bit more complicated as this file gets generated from .in file and there is currently no reliable inter-dir/makefile (siblings) targets dependency tracking in place, AFAICT. --- docs/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 32c6d08f9..2ca1b0ab4 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -29,7 +29,7 @@ dependent_headers = $(subst $(inc_dir),,$(shell \ printf 'include $(inc_dir)/Makefile.am\n\n%%.var:\n\t@echo $$($$*)' \ | ${MAKE} --no-print-directory -f - inst_HEADERS.var \ || echo $(inc_dir)/qb*.h)) -dependent_headers_omit = qbconfig.h qbdefs.h qbrb.h +dependent_headers_omit = qbconfig.h dist_man_MANS += $(patsubst %,man3/%.3,$(filter-out \ $(dependent_headers_omit),$(dependent_headers) \ From 4df20fbb53ccdf01e31411a3d81dc9561fe0b09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Mon, 22 Feb 2016 12:50:46 +0100 Subject: [PATCH 31/33] build: fix man3 pages not installed Issue with original version of the patch spotted by Chrissie (thanks). --- docs/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 2ca1b0ab4..3c46ced88 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -31,11 +31,11 @@ dependent_headers = $(subst $(inc_dir),,$(shell \ || echo $(inc_dir)/qb*.h)) dependent_headers_omit = qbconfig.h -dist_man_MANS += $(patsubst %,man3/%.3,$(filter-out \ +dist_man3_MANS = $(patsubst %,man3/%.3,$(filter-out \ $(dependent_headers_omit),$(dependent_headers) \ )) -$(dist_man_MANS): man.dox $(dependent_headers:%=$(inc_dir)/%) +$(dist_man3_MANS): man.dox $(dependent_headers:%=$(inc_dir)/%) mkdir -p man3 doxygen man.dox From e4909329071e3b3805609e38c535e75f69cf9c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= Date: Mon, 22 Feb 2016 13:11:47 +0100 Subject: [PATCH 32/33] build: GCC < 4.6 does not support diagnostic push/pop pragmas Better to get sort of a spurious warning then to possibly mess up with a warnings safety net. Reported by Chrissie (thanks). --- configure.ac | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 6da1d8034..7cfde10f9 100644 --- a/configure.ac +++ b/configure.ac @@ -506,9 +506,17 @@ if test x"$GCC" = xyes && cc_supports_flag -Wsuggest-attribute=format; then AC_DEFINE([HAVE_GCC_SUGGEST_ATTRIBUTE_FORMAT], [], [gcc supports -Wsuggest-attribute=format]) fi +dnl pretend GCC (<4.6) is not capable of format complaints when it does not +dnl support diagnostic push/pop pragmas (cannot track state reliably, then) if test x"$gcc_format_complaints" = xyes; then - AC_DEFINE([HAVE_GCC_FORMAT_COMPLAINTS], [], - [gcc can complain about missing format attribute]) + backup_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS -Werror" + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +#pragma GCC diagnostic push +#pragma GCC diagnostic pop + ]])], AC_DEFINE([HAVE_GCC_FORMAT_COMPLAINTS], [], + [gcc can complain about missing format attribute])) + CFLAGS="$backup_CFLAGS" fi # --- coverage --- From f439488261ce9bf8604c666bed94ebfc687fd9a2 Mon Sep 17 00:00:00 2001 From: Christine Caulfield Date: Mon, 22 Feb 2016 16:02:25 +0000 Subject: [PATCH 33/33] travis: Add a normal 'make' to the checks make check doesn't build the docs and so errors in that part of the build system will not be checked without a normal 'make'. Signed-off-by: Christine Caulfield --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 6b9a01b00..672fccf4f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ addons: - splint script: ./autogen.sh && ./configure --enable-syslog-tests + && make && make check && make distcheck sudo: false