The last fix to skiplist never ran the code that patched up the level
list as it updated the current level before runnign the loop.
This now works.
Merges: https://github.com/ClusterLabs/libqb/pull/333
Reviewed-by: Jan Pokorný <jpokorny@redhat.com>
* log: Add high-resolution timestamp option for log files
This adds the %T option to the log format for millisecond timestamps. There's a feature test macro QB_FEATURE_LOG_HIRES_TIMESTAMPS so that applications know that they are available.
Because this changes the internal logging API, applications that use custom loggers will also need to change their custom logging destinations to take a struct timespec instead of a time_t. The above feature test macro will help in deciding which is appropriate.
move from AC_PREPROC_IFELSE (strongly discouraged) to AC_COMPILE_IFELSE
our detection system was very weak and recent versions of clang did
show that PREPROC_IFELFE (cpp) would enable warning options that
the compiler does not support (clang).
use a full compilation test to detect what works and what doesn't.
Also expand the warning list to include new / renamed clang options
of equivalents already enabled for older versions of clang and gcc.
Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
* log: add systemd journal as a logging option
systemd journal can be configured as a logging option
at ./configure time (--enable-systemd-journal).
If libqb is buit with this then the syslog target can be switched
to sending to the journal using
qb_log_ctl(QB_LOG_SYSLOG, QB_LOG_CONF_USE_JOURNAL, 1);
As this patch also brought up some locking issues with re-configuring the logging while threaded logging was enabled, it also includes locking around qb_log_ctl2() and conversion of in_logger to an atomic.
(Patch from poki, only committed under my name because github is being
weird)
This used to happen when an iterator contained a reference on the item
to continue with, which got outdated when such item had been removed in
the interim, though it's original memory would still be -- mistakenly --
accessed. Actually such a condition is exercised with an existing
"test_map_iter_safety(ordered=true)" test, though it likely never run
under valgrind's supervision and standard memory checking harness was
too coarse (perhaps because of low memory pressure or other "lucky"
coincidence). Thankfully, the default, paranoid approach towards dynamic
memory handling in OpenBSD (free(3) call makes small chunks "junked",
i.e., filled with 0xdf bytes, see malloc.conf(5)) resulted in the
explicit segmentation fault when tripping over the happens-to-be-freed
pointer in the assumed iteration chain.
We solve the "out-of-sync iterator" issue with a twist, inverting
the responsibility to carry (and more widely, to contribute in the
propagation of) the up-to-date "forward" pointers, as clearly,
iterating over and over through the items would not be very scalable
(and it was not done, which had resulted in the first place).
So now, when any skiplist item is to be removed, its preceding item
gets the "forward" pointers recomputed as before, but then, they are
copied into "forward" pointers for the item to be removed, original
area containing them is disposed, and this preceding item just points
to the area primarily managed by the to-be-removed item (procedure
dubbed "takeover-and-repoint" in the comment). This itself gets
a special mark so that this area won't be dropped when that item gets
disposed, which rather happens with the disposal of the preceding item
that points to the "forward" memory area at hand and is not marked so.
This is believed to be sufficient to address out-of-band (iterator
based) access versus interim future iteration chain mangling, as these
operate de facto on the non-sparse, linear level of the skiplist.
Alternative approaches include:
turning pointers-to-arrays into pointers-to-pointers-to-arrays to
allow for explicit setting to NULL after free, and sharing this
additional indirection -- this straightforward extension was
attempted first, but shortly after, it became apparent it would
be a nightmare with the current interprocedural dependencies
extra tagging of the structures and adding complexities around
checking the eligibility, like every other manipulation with the
skiplist
completely split life-cycle of "node" and "node->forward", i.e.,
separate reference-counting etc.
Also said test was extended to push the corner case to the limit:
when to-resume-with item in the chain is being figured out, the
predecessors may be consulted (it is in that test), but the very
first predecessor is now removed as well, for good measure, as
it makes for boundary condition ^ 2.
Signed-off-by: Jan Pokorný jpokorny@redhat.com
It is my (and several others') opinion that the linker 'magic' used to maintain callsite data in libqb is hugely over complicated and unnecessarily fragile. It's main purpose seems to have been to improve performance but empirical testing shows this to be tiny at best. The overhead of sprintf makes minor optimisations in this code pointless.
With this code removed, libqb allocates callsites using a C static variable at run-time. This sounds bad but in actuality it merely moves the allocation from program load time to the first few milliseconds of program run-time. Applications like corosync and pacemaker spend most of their time in small loops doing the same work over and over again so the overhead doesn't apply and jitter does not occur.
We've tested this with corosync and pacemaker under valgrind and massif and the differences are minimal and even then only show up under artificial stress testing.
For this change I've bumped the soname up to 20 to indicate this is an incompatible change. I'm open to suggestions as to a release number but am currently thinking of 2.0.0
* doc: qbarray.h: fix garbled Doxygen markup
* build: follow-up for and fine-tuning of a rushed 6d62b64 commit
(It made a service as-was, but being afforded more time, this would
have accompanied that commit right away, for better understanding,
brevity and uniformity.)
* build: prune superfluous Makefile declarations within tests directory
There was a significant redundancy wrt. build flags and EXTRA_DIST
assignment (the latter become redundant as of f6e4042 at latest)
spread all over the place (vivat copy&paste). Also, in one instance,
CPPFLAGS (used) was confused with CFLAGS (meant).
* maint: check abi: fix two issues with abi-compliance-checker/libstdc++
1. ABICC >= 2 needs to be passed -cxx-incompatible switch because C is
no longer a default for this tool (used to be vice versa),
plus current version will stop choking on C vs. C++ (our C code with
C++ compatibility wrapping being viewed from C++ perspective for the
purpose of dumping the declared symbols, which somewhat conflicts
with internal masking of the C++ keywords being used as valid C
identifiers [yet some instances must not be masked here, see
https://github.com/lvc/abi-compliance-checker/issues/64) only
if _also_ something like this is applied:
https://github.com/lvc/abi-compliance-checker/pull/70
2. since 20246f5, libqb.so no longer poses a symlink to the actual
version-qualified shared library, but rather a standalone linker
script, which confuses ABICC, so blacklist that file for the scanning
purposes explicitly, together with referring to the library through
it's basic version qualification (which alone, sadly, is not
sufficient as ABICC proceeds to scan whole containing directory
despite particular file is specified)
* maint: check abi: switch to abi-dumper for creating "ABI dumps"
Beside avoiding issues with abi-compliance-checker in the role of ABI
dumps producer (see the preceding commit), it also seems to generate
more accurate picture (maybe because it expressly requires compiling
with debugging information requested).
* Low: qblist.h: fix incompatibility with C++ & check it regularly
* tests: check_list.c: start zeroing in on the gaps in tests' coverage
* tests: print_ver: make preprocessor emit "note" rather than warning
IIRC, Chrissie asked about this around inclusion of the test at
hand, and it seemed there was no way but to emit a warning to get
something output at all. Now it turns wrong, and moreover, we
make the code not fixed on GCC specific pragmas, with a bit of
luck, "#pragma message" approach is adopted more widely by compilers.
Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
* Replace ck_assert_uint_eq() with ck_assert_int_eq()
it's not available in check 0.9
* Proper check for C++ compiler (from Fabio)
* add (c) to copyright dates
This is meant as a lean, customized policy driven alternative
to the original proposal by Jan Friesse <jfriesse@redhat.com> that
balances the inherent trade-off in the opposite direction, so that the
configure.ac script is practically untouched and more weight and policy
is hardcoded in git-version-gen. Problem with that approach stems from
(avoidable) effective fork of the respective gnulib's module and imposed
maintenance burden.
Speaking for libqb in particular, we should nonetheless make it
absolutely clear such in-development snapshots are nothing more,
nothing binding (not to think of viral injection of these bits
into circle of dependent packages upon their rebuilds), since the
changes accumulated since the last official release should only be
assumed firmly committed at the point the new release is cut (may
happen 99% of time with snapshots but no accountability from our
side for the complementary inter-release twists...), which is exactly
when many possibly unanticipated variables like correct SONAME
versions get to reflect what's appropriate.
Also, OpenPGP signature constitutes something more eligible for
one's trust than (provably) bit/content unstable archives without
the possibility of an independent authenticity/integrity verification.
Therefore, the only thinkable and upstream-endorsed use cases
for such snapshots are development-only purposes (CI et al.)!
V2 of the patch:
Thanks to feedback from Jan Friesse, a glitch in "make rpm" et al.
not working with the snapshots was pointed out.
V3:
Only normalize configure.ac back when known to be previously affected
with "git archive" substitution logic, and do not use an exclamation
mark as short commit - decoration separator since that could be
ambiguous (it is a valid branch name character), stick with double
question marks instead (not allowed, doubled for good measure).
Signed-off-by: Jan Pokorný <jpokorny@redhat.com>
Uses dpkg-architecture, if present, to return
DEB_HOST_GNU_TYPE, and use this appended to /usr/include
for form the path.
Signed-off-by: Daniel Black <daniel@linux.ibm.com>
Reviewed-by: Jan Pokorný <jpokorny@redhat.com>
on FreeBSD 11 call dlopen on a shared library causes the constructors
to run again. As we're just getting symbols we don't need this to
happen.
Actually we don't WANT it to happen because it can cause qb_log_init to
be called twice (recursively) and the dlnames list gets corrupted. This
causess corosync (at leasT0 to crash at startup.
Signed-off-by: Christine Caulfield <ccaulfie@redhat.com>
some vendors default to RPATH, others to RUNPATH. The former does not allow override
with LD_LIBRARY_PATH when running binaries such as test suite and it was causing
problems on platforms where RPATH is default, by running the test suite against
the wrong library (out-of-tree vs in-tree).
This change has no effect on libqb itself, but only on the binaries.
Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
* log: Allow flexible size of logging buffer & ellipsis if it overflows.
Allow the logging line length to be changed. Any reasonable length is allowable, the default is 512 as before. Anything more than 512 incurs several mallocs.
Also add an option to set the last 3 characters as '...' if the line length overflows.
Signed-off-by: Christine Caulfield <ccaulfie@redhat.com>
* ipc_shm: Don't truncate SHM files of an active server
I've put in an extra check so that clients don't truncate the
SHM file if the server still exists. Sadly on FreeBSD we can't
get the server PID for the client (unless someone has a patch handy!)
so we still do the truncate when disconnected. As a backstop (and also
to cover the BSD issue) I've added a SIGBUS trap to the server shutdown
so that it doesn't cause a server crash.
Signed-off-by: Christine Caulfield <ccaulfie@redhat.com>
Reviewed by: Jan Friesse <jfriesse@redhat.com>
On Linux systems libqb uses abstract sockets by default, which lack
access control. However, they aren't available on other platforms.
The other option is using file system sockets, by default under
/var/run. This directory is only writable by root, though, which
makes it inapproriate for unprivileged applications. So use /tmp
instead.
See also: https://github.com/ClusterLabs/libqb/issues/294
* tests: Improve test isolation
Make all the IPC tests run with a common date/pid stamp name, so that
the final resource.test only fails if it finds one of OUR files left
lying around and not those from another test.
Falls back to old IPC naming style if we can't create the file.
Signed-off-by: Christine Caulfield <ccaulfie@redhat.com>
Reviewed-by: Jan Friesse <jfriesse@redhat.com>
