From 6e5d2f896365fa6737d0c336b7edbc6b6a15fcb9 Mon Sep 17 00:00:00 2001 From: aszlig Date: Fri, 28 Jul 2017 12:36:48 +0200 Subject: [PATCH] nixos/xserver: Properly validate XKB options Checking the keyboard layout has been a long set of hurdles so far, with several attempts. Originally, the checking was introduced by @lheckemann in #23709. The initial implementation just was trying to check whether the symbols/ directory contained the layout name. Unfortunately, that wasn't enough and keyboard variants weren't recognized, so if you set layout to eg. "dvorak" it will fail with an error (#25526). So my improvement on that was to use sed to filter rules/base.lst and match the layout against that. I fucked up twice with this, first because layout can be a comma-separated list which I didn't account for and second because I ran into a Nix issue (NixOS/nix#1426). After fixing this, it still wasn't enough (and this is btw. what localectl also does), because we were *only* matching rules but not symbols, so using "eu" as a layout won't work either. I decided now it's the time to actually use libxkbcommon to try compiling the keyboard options and see whether it succeeds. This comes in the form of a helper tool called xkbvalidate. IMHO this approach is a lot less error-prone and we can be sure that we don't forget about anything because that's what the X server itself uses to compile the keymap. Another advantage of this is that we now validate the full set of XKB options rather than just the layout. Tested this against a variety of wrong and correct keyboard configurations and against the "keymap" NixOS VM tests. Signed-off-by: aszlig Cc: @lheckemann, @peti, @7c6f434c, @tohl, @vcunat, @lluchs Fixes: #27597 --- nixos/modules/services/x11/xserver.nix | 48 +------- pkgs/tools/X11/xkbvalidate/default.nix | 15 +++ pkgs/tools/X11/xkbvalidate/xkbvalidate.c | 135 +++++++++++++++++++++++ pkgs/top-level/all-packages.nix | 2 + 4 files changed, 156 insertions(+), 44 deletions(-) create mode 100644 pkgs/tools/X11/xkbvalidate/default.nix create mode 100644 pkgs/tools/X11/xkbvalidate/xkbvalidate.c diff --git a/nixos/modules/services/x11/xserver.nix b/nixos/modules/services/x11/xserver.nix index 638509e710b..3ce124d3da2 100644 --- a/nixos/modules/services/x11/xserver.nix +++ b/nixos/modules/services/x11/xserver.nix @@ -648,51 +648,11 @@ in services.xserver.xkbDir = mkDefault "${pkgs.xkeyboard_config}/etc/X11/xkb"; - system.extraDependencies = singleton (pkgs.runCommand "xkb-layouts-exist" { - inherit (cfg) layout xkbDir; + system.extraDependencies = singleton (pkgs.runCommand "xkb-validated" { + inherit (cfg) xkbModel layout xkbVariant xkbOptions; + nativeBuildInputs = [ pkgs.xkbvalidate ]; } '' - # We can use the default IFS here, because the layouts won't contain - # spaces or tabs and are ruled out by the sed expression below. - availableLayouts="$( - sed -n -e ':i /^! \(layout\|variant\) *$/ { - # Loop through all of the layouts/variants until we hit another ! at - # the start of the line or the line is empty ('t' branches only if - # the last substitution was successful, so if the line is empty the - # substition will fail). - :l; n; /^!/bi; s/^ *\([^ ]\+\).*/\1/p; tl - }' "$xkbDir/rules/base.lst" | sort -u - )" - - layoutNotFound() { - echo >&2 - echo "The following layouts and variants are available:" >&2 - echo >&2 - - # While an output width of 80 is more desirable for small terminals, we - # really don't know the amount of columns of the terminal from within - # the builder. The content in $availableLayouts however is pretty - # large, so let's opt for a larger width here, because it will print a - # smaller amount of lines on modern KMS/framebuffer terminals and won't - # lose information even in smaller terminals (it only will look a bit - # ugly). - echo "$availableLayouts" | ${pkgs.utillinux}/bin/column -c 150 >&2 - - echo >&2 - echo "However, the keyboard layout definition in" \ - "\`services.xserver.layout' contains the layout \`$1', which" \ - "isn't a valid layout or variant." >&2 - echo >&2 - exit 1 - } - - # Again, we don't need to take care of IFS, see the comment for - # $availableLayouts. - for l in ''${layout//,/ }; do - if ! echo "$availableLayouts" | grep -qxF "$l"; then - layoutNotFound "$l" - fi - done - + validate "$xkbModel" "$layout" "$xkbVariant" "$xkbOptions" touch "$out" ''); diff --git a/pkgs/tools/X11/xkbvalidate/default.nix b/pkgs/tools/X11/xkbvalidate/default.nix new file mode 100644 index 00000000000..f5a26410835 --- /dev/null +++ b/pkgs/tools/X11/xkbvalidate/default.nix @@ -0,0 +1,15 @@ +{ lib, runCommandCC, libxkbcommon }: + +runCommandCC "xkbvalidate" { + buildInputs = [ libxkbcommon ]; + meta = { + description = "NixOS tool to validate X keyboard configuration"; + license = lib.licenses.mit; + platforms = lib.platforms.linux; + maintainers = [ lib.maintainers.aszlig ]; + }; +} '' + mkdir -p "$out/bin" + gcc -std=gnu11 -Wall -pedantic -lxkbcommon ${./xkbvalidate.c} \ + -o "$out/bin/validate" +'' diff --git a/pkgs/tools/X11/xkbvalidate/xkbvalidate.c b/pkgs/tools/X11/xkbvalidate/xkbvalidate.c new file mode 100644 index 00000000000..d9c9042467c --- /dev/null +++ b/pkgs/tools/X11/xkbvalidate/xkbvalidate.c @@ -0,0 +1,135 @@ +#define _GNU_SOURCE +#include +#include +#include +#include +#include + +#include + +static char **log_buffer = NULL; +static int log_buffer_size = 0; +static bool log_alloc_success = true; + +static void add_log(struct xkb_context *ctx, enum xkb_log_level level, + const char *fmt, va_list args) +{ + log_buffer_size++; + + if (log_buffer == NULL) + log_buffer = malloc(sizeof(char *)); + else + log_buffer = realloc(log_buffer, sizeof(char *) * log_buffer_size); + + if (log_buffer == NULL) { + perror("buffer alloc"); + log_alloc_success = false; + log_buffer_size--; + return; + } + + if (vasprintf(&log_buffer[log_buffer_size - 1], fmt, args) == -1) { + perror("log line alloc"); + log_alloc_success = false; + return; + } +} + +static void print_logs(void) +{ + for (int i = 0; i < log_buffer_size; ++i) + fprintf(stderr, " %s", log_buffer[i]); +} + +static void free_logs(void) +{ + if (log_buffer == NULL) + return; + for (int i = 0; i < log_buffer_size; ++i) + free(log_buffer[i]); + free(log_buffer); + log_buffer = NULL; + log_buffer_size = 0; +} + +static bool try_keymap(struct xkb_context *ctx, struct xkb_rule_names *rdef) +{ + struct xkb_keymap *keymap; + bool result = true; + + if ((keymap = xkb_keymap_new_from_names(ctx, rdef, 0)) == NULL) + result = false; + else + xkb_keymap_unref(keymap); + + return result; +} + +static void print_error(const char *name, const char *value, + const char *nixos_option) +{ + fprintf(stderr, "\nThe value `%s' for keyboard %s is invalid.\n\n" + "Please check the definition in `services.xserver.%s'.\n", + value, name, nixos_option); + fputs("\nDetailed XKB compiler errors:\n\n", stderr); + print_logs(); + putc('\n', stderr); +} + +#define TRY_KEYMAP(name, value, nixos_option) \ + *rdef = (struct xkb_rule_names) {0}; \ + free_logs(); \ + rdef->name = value; \ + result = try_keymap(ctx, rdef); \ + if (!log_alloc_success) \ + goto out; \ + if (!result) { \ + print_error(#name, value, nixos_option); \ + exit_code = EXIT_FAILURE; \ + goto out; \ + } + +int main(int argc, char **argv) +{ + int exit_code = EXIT_SUCCESS; + bool result; + struct xkb_context *ctx; + struct xkb_rule_names *rdef; + + if (argc != 5) { + fprintf(stderr, "Usage: %s model layout variant options\n", argv[0]); + return EXIT_FAILURE; + } + + ctx = xkb_context_new(XKB_CONTEXT_NO_ENVIRONMENT_NAMES); + xkb_context_set_log_fn(ctx, add_log); + + rdef = malloc(sizeof(struct xkb_rule_names)); + + TRY_KEYMAP(model, argv[1], "xkbModel"); + TRY_KEYMAP(layout, argv[2], "layout"); + TRY_KEYMAP(variant, argv[3], "xkbVariant"); + TRY_KEYMAP(options, argv[4], "xkbOptions"); + + free_logs(); + rdef->model = argv[1]; + rdef->layout = argv[2]; + rdef->variant = argv[3]; + rdef->options = argv[4]; + + result = try_keymap(ctx, rdef); + if (!log_alloc_success) + goto out; + + if (!result) { + fputs("The XKB keyboard definition failed to compile:\n", stderr); + print_logs(); + exit_code = EXIT_FAILURE; + } + +out: + free_logs(); + free(rdef); + xkb_context_unref(ctx); + return exit_code; +} diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index e890a4ba197..6fea60980f6 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -4805,6 +4805,8 @@ with pkgs; xbrightness = callPackage ../tools/X11/xbrightness { }; + xkbvalidate = callPackage ../tools/X11/xkbvalidate { }; + xfstests = callPackage ../tools/misc/xfstests { }; xprintidle-ng = callPackage ../tools/X11/xprintidle-ng {};