Messages: merge macros with and without message code

Previously we had two types of macros for logging: with and without
message code. They were intended to be merged afterwards.

The idea is to use a special code – `XKB_LOG_MESSAGE_NO_ID = 0` – that
should *not* be displayed. But we would like to avoid checking this
special code at run time. This is achieved using macro tricks; they
are detailed in the code (see: `PREPEND_MESSAGE_ID`).

Now it is also easier to spot the remaining undocumented log entries:
just search `XKB_LOG_MESSAGE_NO_ID`.

src/compose/parser.c

@@ -727,7 +727,9 @@ parse_file(struct xkb_compose_table *table, FILE *file, const char *file_name)
 
     ok = map_file(file, &string, &size);
     if (!ok) {
-        log_err(table->ctx, "Couldn't read Compose file %s: %s\n",
+        log_err(table->ctx,
+                XKB_LOG_MESSAGE_NO_ID,
+                "Couldn't read Compose file %s: %s\n",
                 file_name, strerror(errno));
         return false;
     }

src/compose/table.c

@@ -206,7 +206,8 @@ xkb_compose_table_new_from_locale(struct xkb_context *ctx,
     }
     free(path);
 
-    log_err(ctx, "couldn't find a Compose file for locale \"%s\" (mapped to \"%s\")\n",
+    log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
+            "couldn't find a Compose file for locale \"%s\" (mapped to \"%s\")\n",
             locale, table->locale);
     xkb_compose_table_unref(table);
     return NULL;
@@ -220,7 +221,8 @@ xkb_compose_table_new_from_locale(struct xkb_context *ctx,
         return NULL;
     }
 
-    log_dbg(ctx, "created compose table from locale %s with path %s\n",
+    log_dbg(ctx, XKB_LOG_MESSAGE_NO_ID,
+            "created compose table from locale %s with path %s\n",
             table->locale, path);
 
     free(path);

src/context-priv.c

@@ -182,6 +182,7 @@ xkb_context_sanitize_rule_names(struct xkb_context *ctx,
         if (!isempty(rmlvo->variant)) {
             const char *variant = xkb_context_get_default_variant(ctx);
             log_warn(ctx,
+                XKB_LOG_MESSAGE_NO_ID,
                 "Layout not provided, but variant set to \"%s\": "
                 "ignoring variant and using defaults for both: "
                 "layout=\"%s\", variant=\"%s\".\n",

src/context.c

@@ -65,13 +65,14 @@ xkb_context_include_path_append(struct xkb_context *ctx, const char *path)
     }
 
     darray_append(ctx->includes, tmp);
-    log_dbg(ctx, "Include path added: %s\n", tmp);
+    log_dbg(ctx, XKB_LOG_MESSAGE_NO_ID, "Include path added: %s\n", tmp);
 
     return 1;
 
 err:
     darray_append(ctx->failed_includes, tmp);
-    log_dbg(ctx, "Include path failed: %s (%s)\n", tmp, strerror(err));
+    log_dbg(ctx, XKB_LOG_MESSAGE_NO_ID,
+            "Include path failed: %s (%s)\n", tmp, strerror(err));
     return 0;
 }
 
@@ -304,7 +305,8 @@ xkb_context_new(enum xkb_context_flags flags)
 
     if (!(flags & XKB_CONTEXT_NO_DEFAULT_INCLUDES) &&
         !xkb_context_include_path_append_default(ctx)) {
-        log_err(ctx, "failed to add default include path %s\n",
+        log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
+                "failed to add default include path %s\n",
                 DFLT_XKB_CONFIG_ROOT);
         xkb_context_unref(ctx);
         return NULL;

src/context.h

@@ -109,46 +109,64 @@ void
 xkb_context_sanitize_rule_names(struct xkb_context *ctx,
                                 struct xkb_rule_names *rmlvo);
 
+/*
+ * Macro sorcery: PREPEND_MESSAGE_ID enables the log functions to format messages
+ * with the message ID only if the ID is not 0 (XKB_LOG_MESSAGE_NO_ID).
+ * This avoid checking the ID value at run time.
+ *
+ * The trick resides in CHECK_ID:
+ * • CHECK_ID(0) expands to:
+ *   ‣ SECOND(MATCH0, WITH_ID, unused)
+ *   ‣ SECOND(unused,WITHOUT_ID, WITH_ID, unused)
+ *   ‣ WITHOUT_ID
+ * • CHECK_ID(123) expands to:
+ *   ‣ SECOND(MATCH123, WITH_ID, unused)
+ *   ‣ WITH_ID
+*/
+#define EXPAND(...)              __VA_ARGS__ /* needed for MSVC compatibility */
+
+#define JOIN_EXPAND(a, b)        a##b
+#define JOIN(a, b)               JOIN_EXPAND(a, b)
+
+#define SECOND_EXPAND(a, b, ...) b
+#define SECOND(...)              EXPAND(SECOND_EXPAND(__VA_ARGS__))
+
+#define MATCH0                   unused,WITHOUT_ID
+#define CHECK_ID(value)          SECOND(JOIN(MATCH, value), WITH_ID, unused)
+
+#define FORMAT_MESSAGE_WITHOUT_ID(id, fmt) fmt
+#define FORMAT_MESSAGE_WITH_ID(id, fmt)    "[XKB-%03d] " fmt, id
+#define PREPEND_MESSAGE_ID(id, fmt) JOIN(FORMAT_MESSAGE_, CHECK_ID(id))(id, fmt)
+
 /*
  * The format is not part of the argument list in order to avoid the
  * "ISO C99 requires rest arguments to be used" warning when only the
  * format is supplied without arguments. Not supplying it would still
  * result in an error, though.
  */
 #define xkb_log_with_code(ctx, level, verbosity, msg_id, fmt, ...) \
-    xkb_log(ctx, level, verbosity, "[XKB-%03d] " fmt, \
-            msg_id, ##__VA_ARGS__)
-#define log_dbg_with_code(ctx, id, ...) \
-    xkb_log_with_code((ctx), XKB_LOG_LEVEL_DEBUG, 0, (id), __VA_ARGS__)
-#define log_dbg(ctx, ...) \
-    xkb_log((ctx), XKB_LOG_LEVEL_DEBUG, 0, __VA_ARGS__)
-#define log_info_with_code(ctx, id, ...) \
-    xkb_log_with_code((ctx), XKB_LOG_LEVEL_INFO, 0, (id), __VA_ARGS__)
-#define log_info(ctx, ...) \
-    xkb_log((ctx), XKB_LOG_LEVEL_INFO, 0, __VA_ARGS__)
-#define log_warn_with_code(ctx, id, ...) \
-    xkb_log_with_code((ctx), XKB_LOG_LEVEL_WARNING, 0, (id), __VA_ARGS__)
-#define log_warn(ctx, ...) \
-    xkb_log((ctx), XKB_LOG_LEVEL_WARNING, 0, __VA_ARGS__)
-#define log_err_with_code(ctx, id, ...) \
-    xkb_log_with_code((ctx), XKB_LOG_LEVEL_ERROR, 0, (id), __VA_ARGS__)
-#define log_err(ctx, ...) \
-    xkb_log((ctx), XKB_LOG_LEVEL_ERROR, 0, __VA_ARGS__)
-#define log_wsgo_with_code(ctx, id, ...) \
-    xkb_log_with_code((ctx), XKB_LOG_LEVEL_CRITICAL, 0, (id), __VA_ARGS__)
-#define log_wsgo(ctx, ...) \
-    xkb_log((ctx), XKB_LOG_LEVEL_CRITICAL, 0, __VA_ARGS__)
+    xkb_log(ctx, level, verbosity, PREPEND_MESSAGE_ID(msg_id, fmt), ##__VA_ARGS__)
+#define log_dbg(ctx, id, ...) \
+    xkb_log_with_code((ctx), XKB_LOG_LEVEL_DEBUG, 0, id, __VA_ARGS__)
+#define log_info(ctx, id, ...) \
+    xkb_log_with_code((ctx), XKB_LOG_LEVEL_INFO, 0, id, __VA_ARGS__)
+#define log_warn(ctx, id, ...) \
+    xkb_log_with_code((ctx), XKB_LOG_LEVEL_WARNING, 0, id, __VA_ARGS__)
+#define log_err(ctx, id, ...) \
+    xkb_log_with_code((ctx), XKB_LOG_LEVEL_ERROR, 0, id, __VA_ARGS__)
+#define log_wsgo(ctx, id, ...) \
+    xkb_log_with_code((ctx), XKB_LOG_LEVEL_CRITICAL, 0, id, __VA_ARGS__)
 #define log_vrb(ctx, vrb, id, ...) \
-    xkb_log_with_code((ctx), XKB_LOG_LEVEL_WARNING, (vrb), (id), __VA_ARGS__)
+    xkb_log_with_code((ctx), XKB_LOG_LEVEL_WARNING, (vrb), id, __VA_ARGS__)
 
 /*
  * Variants which are prefixed by the name of the function they're
  * called from.
  * Here we must have the silly 1 variant.
  */
 #define log_err_func(ctx, fmt, ...) \
-    log_err(ctx, "%s: " fmt, __func__, __VA_ARGS__)
+    log_err(ctx, XKB_LOG_MESSAGE_NO_ID, "%s: " fmt, __func__, __VA_ARGS__)
 #define log_err_func1(ctx, fmt) \
-    log_err(ctx, "%s: " fmt, __func__)
+    log_err(ctx, XKB_LOG_MESSAGE_NO_ID, "%s: " fmt, __func__)
 
 #endif

src/messages-codes.h

@@ -8,17 +8,11 @@
 
 /**
  * Special case when no message identifier is defined.
- *
- * @added 1.6.0
- *
  */
 #define XKB_LOG_MESSAGE_NO_ID 0
 
 /**
  * @name Codes of the log messages
- *
- * @added 1.6.0
- *
  */
 enum xkb_message_code {
     _XKB_LOG_MESSAGE_MIN_CODE = 34,

src/messages-codes.h.jinja

@@ -8,17 +8,11 @@
 
 /**
  * Special case when no message identifier is defined.
- *
- * @added 1.6.0
- *
  */
 #define XKB_LOG_MESSAGE_NO_ID 0
 
 /**
  * @name Codes of the log messages
- *
- * @added 1.6.0
- *
  */
 enum xkb_message_code {
     _XKB_LOG_MESSAGE_MIN_CODE = {{ entries[0].code }},

src/x11/keymap.c

@@ -63,7 +63,7 @@
  */
 #define FAIL_UNLESS(expr) do {                                          \
     if (!(expr)) {                                                      \
-        log_err(keymap->ctx,                                            \
+        log_err(keymap->ctx, XKB_LOG_MESSAGE_NO_ID,                                            \
                 "x11: failed to get keymap from X server: unmet condition in %s(): %s\n", \
                 __func__, STRINGIFY(expr));                             \
         goto fail;                                                      \
@@ -72,7 +72,7 @@
 
 #define FAIL_IF_BAD_REPLY(reply, request_name) do {                     \
     if (!reply) {                                                       \
-        log_err(keymap->ctx,                                            \
+        log_err(keymap->ctx, XKB_LOG_MESSAGE_NO_ID,                                            \
                 "x11: failed to get keymap from X server: %s request failed\n", \
                 (request_name));                                        \
         goto fail;                                                      \

src/xkbcomp/action.c

@@ -194,7 +194,7 @@ ReportMismatch(struct xkb_context *ctx, xkb_message_code_t code,
                enum xkb_action_type action, enum action_field field,
                const char *type)
 {
-    log_err_with_code(ctx, code,
+    log_err(ctx, code,
             "Value of %s field must be of type %s; "
             "Action %s definition ignored\n",
             fieldText(field), type, ActionTypeText(action));
@@ -205,7 +205,7 @@ static inline bool
 ReportIllegal(struct xkb_context *ctx, enum xkb_action_type action,
               enum action_field field)
 {
-    log_err(ctx,
+    log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
             "Field %s is not defined for an action of type %s; "
             "Action definition ignored\n",
             fieldText(field), ActionTypeText(action));
@@ -216,7 +216,7 @@ static inline bool
 ReportActionNotArray(struct xkb_context *ctx, enum xkb_action_type action,
                      enum action_field field)
 {
-    log_err(ctx,
+    log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
             "The %s field in the %s action is not an array; "
             "Action definition ignored\n",
             fieldText(field), ActionTypeText(action));
@@ -423,7 +423,7 @@ HandleMovePtr(struct xkb_context *ctx, const struct xkb_mod_set *mods,
                                   field, "integer");
 
         if (val < INT16_MIN || val > INT16_MAX) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "The %s field in the %s action must be in range %d..%d; "
                     "Action definition ignored\n",
                     fieldText(field), ActionTypeText(action->type),
@@ -470,7 +470,7 @@ HandlePtrBtn(struct xkb_context *ctx, const struct xkb_mod_set *mods,
                                   field, "integer (range 1..5)");
 
         if (btn < 0 || btn > 5) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "Button must specify default or be in the range 1..5; "
                     "Illegal button value %d ignored\n", btn);
             return false;
@@ -495,7 +495,7 @@ HandlePtrBtn(struct xkb_context *ctx, const struct xkb_mod_set *mods,
                                   field, "integer");
 
         if (val < 0 || val > 255) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "The count field must have a value in the range 0..255; "
                     "Illegal count %d ignored\n", val);
             return false;
@@ -555,13 +555,13 @@ HandleSetPtrDflt(struct xkb_context *ctx, const struct xkb_mod_set *mods,
                                   field, "integer (range 1..5)");
 
         if (btn < 0 || btn > 5) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "New default button value must be in the range 1..5; "
                     "Illegal default button value %d ignored\n", btn);
             return false;
         }
         if (btn == 0) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "Cannot set default pointer button to \"default\"; "
                     "Illegal default button setting ignored\n");
             return false;
@@ -603,7 +603,7 @@ HandleSwitchScreen(struct xkb_context *ctx, const struct xkb_mod_set *mods,
                                   field, "integer (0..255)");
 
         if (val < 0 || val > 255) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "Screen index must be in the range 1..255; "
                     "Illegal screen value %d ignored\n", val);
             return false;
@@ -667,7 +667,7 @@ HandlePrivate(struct xkb_context *ctx, const struct xkb_mod_set *mods,
                                   ACTION_TYPE_PRIVATE, field, "integer");
 
         if (type < 0 || type > 255) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "Private action type must be in the range 0..255; "
                     "Illegal type %d ignored\n", type);
             return false;
@@ -684,7 +684,7 @@ HandlePrivate(struct xkb_context *ctx, const struct xkb_mod_set *mods,
          * make actions like these no-ops for now.
          */
         if (type < ACTION_TYPE_PRIVATE) {
-            log_info(ctx,
+            log_info(ctx, XKB_LOG_MESSAGE_NO_ID,
                      "Private actions of type %s are not supported; Ignored\n",
                      ActionTypeText(type));
             act->type = ACTION_TYPE_NONE;
@@ -708,7 +708,7 @@ HandlePrivate(struct xkb_context *ctx, const struct xkb_mod_set *mods,
             str = xkb_atom_text(ctx, val);
             len = strlen(str);
             if (len < 1 || len > sizeof(act->data)) {
-                log_warn(ctx,
+                log_warn(ctx, XKB_LOG_MESSAGE_NO_ID,
                          "A private action has %ld data bytes; "
                          "Illegal data ignored\n", sizeof(act->data));
                 return false;
@@ -723,14 +723,14 @@ HandlePrivate(struct xkb_context *ctx, const struct xkb_mod_set *mods,
             int ndx, datum;
 
             if (!ExprResolveInteger(ctx, array_ndx, &ndx)) {
-                log_err(ctx,
+                log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                         "Array subscript must be integer; "
                         "Illegal subscript ignored\n");
                 return false;
             }
 
             if (ndx < 0 || (size_t) ndx >= sizeof(act->data)) {
-                log_err(ctx,
+                log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                         "The data for a private action is %lu bytes long; "
                         "Attempt to use data[%d] ignored\n",
                         (unsigned long) sizeof(act->data), ndx);
@@ -742,7 +742,7 @@ HandlePrivate(struct xkb_context *ctx, const struct xkb_mod_set *mods,
                                       field, "integer");
 
             if (datum < 0 || datum > 255) {
-                log_err(ctx,
+                log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                         "All data for a private action must be 0..255; "
                         "Illegal datum %d ignored\n", datum);
                 return false;
@@ -794,14 +794,15 @@ HandleActionDef(struct xkb_context *ctx, ActionsInfo *info,
     enum xkb_action_type handler_type;
 
     if (def->expr.op != EXPR_ACTION_DECL) {
-        log_err(ctx, "Expected an action definition, found %s\n",
+        log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
+                "Expected an action definition, found %s\n",
                 expr_op_type_to_string(def->expr.op));
         return false;
     }
 
     str = xkb_atom_text(ctx, def->action.name);
     if (!stringToAction(str, &handler_type)) {
-        log_err(ctx, "Unknown action %s\n", str);
+        log_err(ctx, XKB_LOG_MESSAGE_NO_ID, "Unknown action %s\n", str);
         return false;
     }
 
@@ -841,15 +842,16 @@ HandleActionDef(struct xkb_context *ctx, ActionsInfo *info,
             return false;
 
         if (elemRtrn) {
-            log_err(ctx,
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
                     "Cannot change defaults in an action definition; "
                     "Ignoring attempt to change %s.%s\n",
                     elemRtrn, fieldRtrn);
             return false;
         }
 
         if (!stringToField(fieldRtrn, &fieldNdx)) {
-            log_err(ctx, "Unknown field name %s\n", fieldRtrn);
+            log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
+                    "Unknown field name %s\n", fieldRtrn);
             return false;
         }
 
@@ -873,7 +875,8 @@ SetActionField(struct xkb_context *ctx, ActionsInfo *info,
         return false;
 
     if (!stringToField(field, &action_field)) {
-        log_err(ctx, "\"%s\" is not a legal field name\n", field);
+        log_err(ctx, XKB_LOG_MESSAGE_NO_ID,
+                "\"%s\" is not a legal field name\n", field);
         return false;
     }
 

src/xkbcomp/ast-build.c

@@ -525,7 +525,7 @@ IncludeCreate(struct xkb_context *ctx, char *str, enum merge_mode merge)
     return first;
 
 err:
-    log_err_with_code(ctx,
+    log_err(ctx,
             XKB_ERROR_INVALID_INCLUDE_STATEMENT,
             "Illegal include statement \"%s\"; Ignored\n", stmt);
     FreeInclude(first);