libqb/01-Med-qblog.h-better-explanation-behaviour-of-QB_LOG_I.patch

From 47f007a9bbdbfcdb1df6736dddde64f9c6381b58 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Pokorn=C3=BD?= <jpokorny@redhat.com>
Date: Fri, 6 Oct 2017 17:17:26 +0200
Subject: [PATCH 1/6] Med: qblog.h: better explanation + behaviour of
 QB_LOG_INIT_DATA
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Based on better understanding how link-time callsite collection works,
put a better description for the macro.  Also based on poor user
experience in case that feature does not work well, say because
the linker deliberately changes the previously settled visibility
of the section boundary symbols (happened in ld from binutils-2.29,
fix is forthcoming), tweak the assertion message a bit, together
with an extension of the general intro to point that macro out.
Also play fair, include the <assert.h> header, which this macro
requires.

- use case:
  /usr/sbin/pacemakerd --features

- before:
  pacemakerd: utils.c:69: common:
  Assertion `0' failed

- after:
  pacemakerd: utils.c:69: common:
  Assertion `"non-empty implicit callsite section" && QB_ATTR_SECTION_START != QB_ATTR_SECTION_STOP' failed.

Restructuring of the assertion inspired by the suggestion of Ferenc
Wágner (for the subsequent commit, actually).

And regarding:
> as a side effect, it can ensure the boundary-denoting symbols for
> the target collection area are kept alive with some otherwise unkind
> linkers
this was actually empirically discovered in one particular combination
with ld.bfd @ binutils 2.29, and extensively with 2.29.1 (placing here
a forward reference for the following commits that elaborate on the
libqb-target cross-combination matrix and the findings).

Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
---
 include/qb/qblog.h | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/qb/qblog.h b/include/qb/qblog.h
index 3cb4eef..4954932 100644
--- a/include/qb/qblog.h
+++ b/include/qb/qblog.h
@@ -42,6 +42,10 @@ extern "C" {
 #undef QB_HAVE_ATTRIBUTE_SECTION
 #endif /* S_SPLINT_S */
 
+#ifdef QB_HAVE_ATTRIBUTE_SECTION
+#include <assert.h>  /* possibly needed for QB_LOG_INIT_DATA */
+#endif
+
 /**
  * @file qblog.h
  * The logging API provides four main parts (basics, filtering, threading & blackbox).
@@ -64,6 +68,17 @@ extern "C" {
  * }
  * @endcode
  *
+ * @note
+ * In practice, such a minimalistic approach hardly caters real use cases.
+ * Following section discusses the customization.  Moreover, it's quite
+ * vital to instrument the target user of this logging subsystem with
+ * @c QB_LOG_INIT_DATA() macro placed in the top file scope in an exactly
+ * one source file (preferably the main one) to be mixed into the resulting
+ * compilation unit.  This is a self-defensive measure for when the
+ * linker-assisted collection of callsite data silently fails, which
+ * could otherwise go unnoticed, causing troubles down the road.
+ *
+ *
  * @par Configuring log targets.
  * A log target can be syslog, stderr, the blackbox, stdout, or a text file.
  * By default only syslog is enabled.
@@ -268,11 +283,31 @@ typedef void (*qb_log_filter_fn)(struct qb_log_callsite * cs);
 extern struct qb_log_callsite QB_ATTR_SECTION_START[];
 extern struct qb_log_callsite QB_ATTR_SECTION_STOP[];
 
-/* mere linker sanity check, possible future extension for internal purposes */
-#define QB_LOG_INIT_DATA(name)							\
-    void name(void);								\
-    void name(void) 								\
-    { if (QB_ATTR_SECTION_START == QB_ATTR_SECTION_STOP) assert(0); }	\
+/* optional on-demand self-check of 1/ toolchain sanity (prerequisite for
+   the logging subsystem to work properly) and 2/ non-void active use of
+   logging (satisfied with a justifying existence of a logging callsite as
+   defined with a @c qb_logt invocation) at the target (but see below), which
+   is supposedly assured by it's author(!) as of relying on this very macro
+   [technically, the symbols that happen to be resolved under the respective
+   identifiers do not necessarily originate in the same compilation unit as
+   when it's not the end executable (or by induction, a library positioned
+   earlier in the symbol lookup order) but a shared library, the former takes
+   a precedence unless that site comes short of exercising the logging,
+   making its callsite section empty and, in turn, without such boundary
+   symbols, hence making the resolution continue further in the lookup order
+   -- despite fuzzily targeted attestation, the check remains reasonable];
+   only effective when link-time ("run-time amortizing") callsite collection
+   is;  as a side effect, it can ensure the boundary-denoting symbols for the
+   target collection area are kept alive with some otherwise unkind linkers;
+   may be extended in future for more in-depth self-validation */
+#define QB_LOG_INIT_DATA(name)						\
+    void name(void);							\
+    void name(void) {							\
+    /* our own (target's) sanity, or possibly that of higher priority	\
+       symbol resolution site (unless target equals end executable)	\
+       or even the lower one if no such predecessor defines these */	\
+    assert("non-empty implicit callsite section"			\
+             && QB_ATTR_SECTION_START != QB_ATTR_SECTION_STOP); }	\
     void __attribute__ ((constructor)) name(void);
 #else
 #define QB_LOG_INIT_DATA(name)
-- 
2.15.0