Skip to content

Commit

Permalink
Make it backward compatible
Browse files Browse the repository at this point in the history
  • Loading branch information
root authored and root committed Jan 19, 2021
1 parent c6997df commit 48b9bcf
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 42 deletions.
9 changes: 9 additions & 0 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,16 @@ AC_SUBST([rt_LIBS],[$LIBS])
AX_RESTORE_FLAGS

USE_JOURNAL="no"
QB_SUPPORTS_MESSAGE_IDS="no"
PKG_CHECK_MODULES([SYSTEMD], [libsystemd], [have_systemd=yes],[have_systemd=no])
if test "x$enable_systemd_journal" = "xyes" ; then
if test "x$have_systemd" = "xyes" ; then
AC_DEFINE_UNQUOTED(USE_JOURNAL, 1, [Use systemd journal logging])
USE_JOURNAL="yes"
if test "x$enable_systemd_catalogs" = "xyes" ; then
AC_DEFINE_UNQUOTED(QB_SUPPORTS_MESSAGE_IDS, 1, [Use systemd catalogs])
QB_SUPPORTS_MESSAGE_IDS="yes"
fi
else
echo "systemd libraries not found, will just use syslog"
fi
Expand Down Expand Up @@ -572,6 +577,9 @@ AC_ARG_WITH([socket-dir],
AC_ARG_ENABLE([systemd-journal],
[AS_HELP_STRING([--enable-systemd-journal],[Allow use of systemd journal instead of syslog])])

AC_ARG_ENABLE([systemd-catalogs],
[AS_HELP_STRING([--enable-systemd-catalogs],[Allow use of systemd catalogs. Use together with --enable-systemd-journal])])

AC_ARG_WITH([force-sockets-config-file],
[AS_HELP_STRING([--with-force-sockets-config-file=FILE],[config file to force IPC to use filesystem sockets (Linux & Cygwin only) @<:@SYSCONFDIR/libqb/force-filesystem-sockets@:>@])],
[ FORCESOCKETSFILE="$withval" ],
Expand Down Expand Up @@ -875,6 +883,7 @@ AC_MSG_RESULT([ System configuration = ${sysconfdir}])
AC_MSG_RESULT([ SOCKETDIR = ${SOCKETDIR}])
AC_MSG_RESULT([ Features = ${PACKAGE_FEATURES}])
AC_MSG_RESULT([ Use systemd journal = ${USE_JOURNAL}])
AC_MSG_RESULT([ Use systemd catalogs = ${QB_SUPPORTS_MESSAGE_IDS}])
AC_MSG_RESULT([])
AC_MSG_RESULT([$PACKAGE build info:])
AC_MSG_RESULT([ Optimization = ${OPT_CFLAGS}])
Expand Down
40 changes: 24 additions & 16 deletions include/qb/qblog.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,9 @@ typedef const char *(*qb_log_tags_stringify_fn)(uint32_t tags);
* with the message-id
*/
struct qb_log_callsite {
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id;
#endif
const char *function;
const char *filename;
const char *format;
Expand Down Expand Up @@ -315,15 +317,21 @@ void qb_log_from_external_source(const char *function,
* @param lineno file line number
* @param tags the tag
*/
struct qb_log_callsite* qb_log_callsite_get(const char *message_id,
struct qb_log_callsite* qb_log_callsite_get(
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id,
#endif
const char *function,
const char *filename,
const char *format,
uint8_t priority,
uint32_t lineno,
uint32_t tags);

void qb_log_from_external_source_va2(const char *message_id,
void qb_log_from_external_source_va(
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id,
#endif
const char *function,
const char *filename,
const char *format,
Expand All @@ -333,15 +341,6 @@ void qb_log_from_external_source_va2(const char *message_id,
va_list ap)
__attribute__ ((format (printf, 3, 0)));

void qb_log_from_external_source_va(const char *function,
const char *filename,
const char *format,
uint8_t priority,
uint32_t lineno,
uint32_t tags,
va_list ap)
__attribute__ ((format (printf, 3, 0)));

/**
* This is the function to generate a log message if you want to
* manually add tags.
Expand All @@ -354,12 +353,21 @@ void qb_log_from_external_source_va(const char *function,
* @param fmt usual printf style format specifiers
* @param args usual printf style args
*/
#define qb_logt(priority, tags, fmt, args...) do { \
struct qb_log_callsite* descriptor_pt = \
qb_log_callsite_get(NULL, __func__, __FILE__, fmt, \
priority, __LINE__, tags); \
qb_log_real_(descriptor_pt, ##args); \
#ifdef QB_SUPPORTS_MESSAGE_IDS
#define qb_logt(priority, tags, fmt, args...) do { \
struct qb_log_callsite* descriptor_pt = \
qb_log_callsite_get(NULL, __func__, __FILE__, fmt, \
priority, __LINE__, tags); \
qb_log_real_(descriptor_pt, ##args); \
} while(0)
#else
#define qb_logt(priority, tags, fmt, args...) do { \
struct qb_log_callsite* descriptor_pt = \
qb_log_callsite_get(__func__, __FILE__, fmt, \
priority, __LINE__, tags); \
qb_log_real_(descriptor_pt, ##args); \
} while(0)
#endif


/**
Expand Down
41 changes: 23 additions & 18 deletions lib/log.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,10 @@ qb_log_thread_log_write(struct qb_log_callsite *cs,
}

struct qb_log_callsite*
qb_log_callsite_get(const char *message_id,
qb_log_callsite_get(
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id,
#endif
const char *function,
const char *filename,
const char *format,
Expand All @@ -336,8 +339,11 @@ qb_log_callsite_get(const char *message_id,
return NULL;
}

cs = qb_log_dcs_get(&new_dcs, message_id, function, filename,
format, priority, lineno, tags);
cs = qb_log_dcs_get(&new_dcs,
#ifdef QB_SUPPORTS_MESSAGE_IDS
message_id,
#endif
function, filename, format, priority, lineno, tags);
if (cs == NULL) {
return NULL;
}
Expand Down Expand Up @@ -382,7 +388,10 @@ qb_log_callsite_get(const char *message_id,
}

void
qb_log_from_external_source_va2(const char *message_id,
qb_log_from_external_source_va(
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id,
#endif
const char *function,
const char *filename,
const char *format,
Expand All @@ -395,22 +404,14 @@ qb_log_from_external_source_va2(const char *message_id,
return;
}

cs = qb_log_callsite_get(message_id, function, filename,
format, priority, lineno, tags);
cs = qb_log_callsite_get(
#ifdef QB_SUPPORTS_MESSAGE_IDS
message_id,
#endif
function, filename, format, priority, lineno, tags);
qb_log_real_va_(cs, ap);
}

void
qb_log_from_external_source_va(const char *function,
const char *filename,
const char *format,
uint8_t priority,
uint32_t lineno, uint32_t tags, va_list ap)
{
qb_log_from_external_source_va2(NULL, function, filename,
format, priority, lineno, tags, ap);
}

void
qb_log_from_external_source(const char *function,
const char *filename,
Expand All @@ -425,7 +426,11 @@ qb_log_from_external_source(const char *function,
return;
}

cs = qb_log_callsite_get(NULL, function, filename,
cs = qb_log_callsite_get(
#ifdef QB_SUPPORTS_MESSAGE_IDS
NULL,
#endif
function, filename,
format, priority, lineno, tags);
va_start(ap, tags);
qb_log_real_va_(cs, ap);
Expand Down
32 changes: 25 additions & 7 deletions lib/log_dcs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ _log_register_callsites(qb_array_t * a, uint32_t bin)
}

static struct qb_log_callsite *
_log_dcs_new_cs(const char *message_id,
_log_dcs_new_cs(
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id,
#endif
const char *function,
const char *filename,
const char *format,
Expand All @@ -71,7 +74,9 @@ _log_dcs_new_cs(const char *message_id,
assert(rc == 0);
assert(cs != NULL);

#ifdef QB_SUPPORTS_MESSAGE_IDS
cs->message_id = strdup(message_id);
#endif
cs->function = strdup(function);
cs->filename = strdup(filename);
cs->format = strdup(format);
Expand All @@ -84,7 +89,9 @@ _log_dcs_new_cs(const char *message_id,

struct qb_log_callsite *
qb_log_dcs_get(int32_t * newly_created,
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id,
#endif
const char *function,
const char *filename,
const char *format,
Expand All @@ -95,14 +102,17 @@ qb_log_dcs_get(int32_t * newly_created,
struct callsite_list *csl_head;
struct callsite_list *csl_last = NULL;
struct callsite_list *csl;
const char *safe_message_id = message_id;
const char *safe_filename = filename;
const char *safe_function = function;
const char *safe_format = format;

#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *safe_message_id = message_id;
if (message_id == NULL) {
safe_message_id = "";
}
#endif

if (filename == NULL) {
safe_filename = "";
}
Expand All @@ -127,7 +137,9 @@ qb_log_dcs_get(int32_t * newly_created,
(void)qb_thread_lock(arr_next_lock);
if (csl_head->cs &&
priority == csl_head->cs->priority &&
#ifdef QB_SUPPORTS_MESSAGE_IDS
strcmp(safe_message_id, csl_head->cs->message_id) == 0 &&
#endif
strcmp(safe_filename, csl_head->cs->filename) == 0 &&
strcmp(safe_format, csl_head->cs->format) == 0) {
(void)qb_thread_unlock(arr_next_lock);
Expand All @@ -138,7 +150,11 @@ qb_log_dcs_get(int32_t * newly_created,
* so we will either have to create it or go through a list
*/
if (csl_head->cs == NULL) {
csl_head->cs = _log_dcs_new_cs(safe_message_id, safe_function,
csl_head->cs = _log_dcs_new_cs(
#ifdef QB_SUPPORTS_MESSAGE_IDS
safe_message_id,
#endif
safe_function,
safe_filename, safe_format,
priority, lineno, tags);
cs = csl_head->cs;
Expand All @@ -149,9 +165,7 @@ qb_log_dcs_get(int32_t * newly_created,
assert(csl->cs->lineno == lineno);
if (priority == csl->cs->priority &&
strcmp(safe_format, csl->cs->format) == 0 &&
strcmp(safe_filename, csl->cs->filename) == 0 &&
strcmp(safe_message_id, csl->cs->message_id) == 0)
{
strcmp(safe_filename, csl->cs->filename) == 0) {
cs = csl->cs;
break;
}
Expand All @@ -163,7 +177,11 @@ qb_log_dcs_get(int32_t * newly_created,
if (csl == NULL) {
goto cleanup;
}
csl->cs = _log_dcs_new_cs(safe_message_id, safe_function,
csl->cs = _log_dcs_new_cs(
#ifdef QB_SUPPORTS_MESSAGE_IDS
safe_message_id,
#endif
safe_function,
safe_filename, safe_format,
priority, lineno, tags);
csl->next = NULL;
Expand Down
2 changes: 2 additions & 0 deletions lib/log_int.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ void qb_log_thread_resume(struct qb_log_target *t);
void qb_log_dcs_init(void);
void qb_log_dcs_fini(void);
struct qb_log_callsite *qb_log_dcs_get(int32_t *newly_created,
#ifdef QB_SUPPORTS_MESSAGE_IDS
const char *message_id,
#endif
const char *function,
const char *filename,
const char *format,
Expand Down
5 changes: 4 additions & 1 deletion lib/log_syslog.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ _syslog_logger(int32_t target,
}
#ifdef USE_JOURNAL
if (t->use_journal) {
sd_journal_send("MESSAGE_ID=%s", cs->message_id,
sd_journal_send(
#ifdef QB_SUPPORTS_MESSAGE_IDS
"MESSAGE_ID=%s", cs->message_id,
#endif
"PRIORITY=%d", final_priority,
"CODE_LINE=%d", cs->lineno,
"CODE_FILE=%s", cs->filename,
Expand Down

2 comments on commit 48b9bcf

@chrissie-c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately that still breaks backwards compatibility. The signature of qb_log_callsite_get() can't be changed because it's already embedded in binaries that have linked with the existing qblog.h file - it's in the qb_logt (and hence also qb_log) macro. So there would have to be a new call that had the message ID in it which is called by new binaries and the exiting one left untouched that, probably, calls the new one with a NULL message_id.

@kgaillot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, libqb instances built with and without QB_SUPPORTS_MESSAGE_IDS would have the same shared object version, so all clients would need to use the ifdef correctly with every affected call or face a crash (and obviously existing clients don't do that). So it would actually be better to make the change unconditionally and bump the shared object version, than to make a public API interface depend on a condition. However in this case we have a way to preserve backward compatibility so that's even better.

Please sign in to comment.