Add errno like mechanism for querying errors

[Caellian]

Most errors & warnings are reported using xkb_context_set_log_fn. This works well only for tools in this repo and doesn't provide any actionable information to applications that aren't CLIs.

Many applications that rely on libxkbcommon will not trickle down these errors to the user because it's likely their fault libxkbcommon function failed instead of user error. But none of the failures are recoverable (in practice), because code has to rely on parsing the errors from xkb_context_set_log_fn to know "I used a flag that's not supported by system libxkbcommon", or "the keymap source is malformed".

Returning null works well only if there is a single failure condition.

Further xkb_context_set_log_fn isn't usable directly from Rust FFI anyway because of vararg. It will be in future when c_variadic lands, but until then, I'd have to use a trampoline function in C, to get Rust side to handle xkb_context_set_log_fn + global static mutex + text pattern matching to return Result<Context, ContextError>.

Suggestion

Make all functions that can fail set some errno like static so that code can check the failure condition using xkb_get_error without having to parse human-readable strings. I'm not suggesting errno because only 3/all error values apply and you'd have more freedom if you defined your own enum in xkbcommon-error.h.

Error logging macro could take this additional error code and set the global on its own.

I'm assuming only a single error condition can occur, though it could be a null padded array and there could be an additional one for warnings - depending on your preferences.

[Caellian]

For context, here's an example of a trampoline necessary for xkb_context_set_log_fn in Rust:

#include <stdio.h>
#include <stdlib.h>
#include <xkbcommon/xkbcommon.h>

typedef void (*trampoline_t)(struct xkb_context *context, enum xkb_log_level level, const char *message);
static trampoline_t trampoline = NULL;

void handler(struct xkb_context *context, enum xkb_log_level level, const char *format, va_list args) {
    size_t len = sprintf(NULL, format, args);
    char *output = malloc(len);
    sprintf(output, format, args);
    trampoline(context, level, output);
    free(output);
}

void context_set_log_fn_trampoline(struct xkb_context *context,
    void (*callback)(struct xkb_context *context, enum xkb_log_level level, const char *message)) {
    trampoline = callback;
    xkb_context_set_log_fn(context, handler);
}

Standard handler is simply not constructable and I believe it's UB to pass a function without va_list.

[wismill]

NOTE: the error message were never meant to be parsable beyond the message identifier XKB-nnn at the start.

A better error handling would be great, but I would rather avoid an errno-like mechanism and prefer passing an error ID directly to the function by reference. The issue is that it means duplicating all relevant functions, which is also not a great idea now. The new functions should however favor returning at least an error ID when relevant.

When #942 get merged, I am not sure such an error mechanism will be so important anymore: while testing available features via flags enables to avoid/recover from errors, the other errors are most probably not recoverable anyway (allocation error, invalid keymap, missing include directories, etc.).

[Caellian]

Most functions in xkbcommon have only a small number of failure modes, and for many callers a NULL return is “good enough” given #942. The API is also mature, so it’s unlikely that a bunch of new error states will appear. But even with a small error surface, having a structured error value remains useful because it lets callers reliably distinguish between the few cases that do matter. It also allows you to expand without fear in future, and to add more details to error values. E.g. you can reserve a ranges of error values and then later return precisely why scanner failed to parse the keymap.

Out-of-memory is the clearest example, though it's often overlooked (even in Rust stdlib for now). Without an explicit error code, callers have to assume the worst or continue executing code paths that were already invalidated by the first allocation failure. That is manageable in simple programs, but becomes fragile in toolkits, compositors, and bindings that wrap xkbcommon inside larger subsystems. You don't want a WM to crash if keymap is invalid, but OOM means it must; instead of loading some default to allow the user to open a terminal and fix their keymap - so in practice WMs will defer handling OOM until some later call and that makes their code messy.

As for not liking errno - there's 3 ways to deal with it the way I see it:

• Return value is a struct {is_ok_bool, ok_value, err_value}, which feels clunky on C side due to lack of automatic cleanup,
• Argument pointer - requires duplicating entire API as you mentioned,
• And errno-like mechanism.

A single per-context “last error” enum value would preserve the existing API and avoid duplication, while still giving both C users and bindings the ability to handle the few important error cases correctly. It seems like most libraries opt for errno approach (OpenGL, Vulkan, GStreamer, Wayland, SDL, ...), and I believe it's what feels most unencumbered in C.

GStreamer associates the error with objects, which is what gave me the idea to actually store the error state in Context instead of a global static, this means that multiple concurrent Contexts can handle errors separately (though I doubt anyone would need that in practice).

[whot]

For context, here's an example of a trampoline necessary for xkb_context_set_log_fn in Rust:

Side-note: replacing the log handler in a future xkbcommon version with a non-vararg function that takes a const char *message and simply printing to a buffer inside libxkbcommon (i.e. your trampoline function) would be totally possible. I reckon the vararg approach dates back to a decade where not printing into a buffer was prone to allocation issues, those times have long since gone.