[PATCH v2] Xorg: Add a suid root wrapper
Hans de Goede
hdegoede at redhat.com
Tue Mar 11 01:24:33 PDT 2014
Hi,
On 03/11/2014 07:16 AM, Peter Hutterer wrote:
> On Mon, Mar 10, 2014 at 10:21:39PM +0100, Hans de Goede wrote:
>> With the recent systemd-logind changes it is possible to install the Xorg
>> binary without suid root rights and still have everything working as it
>> should *if* the user only has cards which are supported by kms.
>>
>> This commit adds a little suid root wrapper, which is a bit weird, first we
>> strip the suid-root bit of the Xorg binary, and then we add a wrapper ?
>>
>> The function of this wrapper is to see if a system still needs root-rights,
>> if it does not (it supports kms and the kms drivers are properly loaded),
>> then it will immediately drop all elevated rights before executing the real
>> Xorg binary. If it finds (some) cards which don't support kms, or no cards
>> at all, then it will execute the Xorg server with elevated rights so that
>> ie the nvidia binary driver and the vesa driver can keep working normally.
>>
>> To make it possible for security concious users who don't need the root
>> rights to completely remove the wrapper, Xorg is started in a 3 step process
>> when the wrapper is enabled during build time:
>>
>> 1) A simple shell script which checks if the wrapper is there, if it is
>> it executes the wrapper, if not it directly executes the real Xorg binary
>>
>> 2) The wrapper gets executed, does its checks, normally drops all elevated
>> rights and then executes the real Xorg binary
>>
>> 3) The real Xorg binary does its thing
>>
>> This allows distributions to put the wrapper binary in a separate package, and
>> will allow users to remove this package. IE the plan with Fedora is to make
>> "legacy" drivers depend on the wrapper pkg, and since our default install
>> contains some legacy drivers it will be part of the default install, but
>> users can later yum remove it (which will also automatically remove the
>> legacy driver packages as those won't work without it anyways).
>>
>> The wrapper is loosely modelled after the existing Debian Xwrapper, it
>> uses the same config-file + config-file format, and also allows restricting
>> Xserver execution (through the wrapper) to console users only.
>>
>> There also is a new needs_root_rights config file directive, which can
>> be used to override the auto-detection the wrapper does.
>>
>> Hopefully this will allow Debian to replace their own wrapper with this
>> upstream one.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>> configure.ac | 14 ++-
>> hw/xfree86/Makefile.am | 15 ++-
>> hw/xfree86/Xorg.sh.in | 12 ++
>> hw/xfree86/man/Makefile.am | 5 +
>> hw/xfree86/man/Xorg.wrap.man | 51 ++++++++
>> hw/xfree86/man/Xwrapper.config.man | 66 +++++++++++
>> hw/xfree86/xorg-wrapper.c | 231 +++++++++++++++++++++++++++++++++++++
>> include/dix-config.h.in | 6 +
>> include/xwin-config.h.in | 3 -
>> manpages.am | 1 +
>> 10 files changed, 398 insertions(+), 6 deletions(-)
>> create mode 100644 hw/xfree86/Xorg.sh.in
>> create mode 100644 hw/xfree86/man/Xorg.wrap.man
>> create mode 100644 hw/xfree86/man/Xwrapper.config.man
>> create mode 100644 hw/xfree86/xorg-wrapper.c
>>
>> diff --git a/configure.ac b/configure.ac
>> index 162c0cf..749fa84 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -627,6 +627,7 @@ AC_ARG_ENABLE(pciaccess, AS_HELP_STRING([--enable-pciaccess], [Build Xorg with p
>> AC_ARG_ENABLE(linux_acpi, AS_HELP_STRING([--disable-linux-acpi], [Disable building ACPI support on Linux (if available).]), [enable_linux_acpi=$enableval], [enable_linux_acpi=yes])
>> AC_ARG_ENABLE(linux_apm, AS_HELP_STRING([--disable-linux-apm], [Disable building APM support on Linux (if available).]), [enable_linux_apm=$enableval], [enable_linux_apm=yes])
>> AC_ARG_ENABLE(systemd-logind, AC_HELP_STRING([--enable-systemd-logind], [Build systemd-logind support (default: auto)]), [SYSTEMD_LOGIND=$enableval], [SYSTEMD_LOGIND=auto])
>> +AC_ARG_ENABLE(suid-wrapper, AC_HELP_STRING([--enable-suid-wrapper], [Build suid-root wrapper for legacy driver support on rootless xserver systems (default: no)]), [SUID_WRAPPER=$enableval], [SUID_WRAPPER=no])
>>
>> dnl DDXes.
>> AC_ARG_ENABLE(xorg, AS_HELP_STRING([--enable-xorg], [Build Xorg server (default: auto)]), [XORG=$enableval], [XORG=auto])
>> @@ -924,6 +925,16 @@ if test "x$SYSTEMD_LOGIND" = xyes; then
>> fi
>> AM_CONDITIONAL(SYSTEMD_LOGIND, [test "x$SYSTEMD_LOGIND" = xyes])
>>
>> +if test "x$SUID_WRAPPER" = xyes; then
>> + dnl The wrapper uses libdrm headers, so ensure they are available
>> + PKG_CHECK_MODULES([LIBDRM], $LIBDRM)
>> + dnl This is a define so that if some platforms want to put the wrapper
>> + dnl somewhere else this can be easily changed
>> + AC_DEFINE_DIR(SUID_WRAPPER_DIR, libexecdir, [Where to install Xorg.bin and Xorg.wrap])
>> + SETUID="no"
>> +fi
>> +AM_CONDITIONAL(SUID_WRAPPER, [test "x$SUID_WRAPPER" = xyes])
>> +
>> if test "x$NEED_DBUS" = xyes; then
>> AC_DEFINE(NEED_DBUS, 1, [Enable D-Bus core])
>> fi
>> @@ -2116,7 +2127,6 @@ fi
>> AC_MSG_RESULT([$XWIN])
>>
>> if test "x$XWIN" = xyes; then
>> - AC_DEFINE_DIR(SYSCONFDIR, sysconfdir, [Location of system.XWinrc])
>> AC_DEFINE_DIR(DEFAULT_LOGDIR, logdir, [Default log location])
>> AC_DEFINE_UNQUOTED(XORG_VERSION_CURRENT, [$VENDOR_RELEASE], [Current Xorg version])
>> AC_DEFINE_UNQUOTED(__VENDORDWEBSUPPORT__, ["$VENDOR_WEB"], [Vendor web address for support])
>> @@ -2440,6 +2450,7 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
>> [ Enable GNU and other extensions to the C environment for glibc])])
>>
>> AC_DEFINE_DIR(PROJECTROOT, prefix, [Overall prefix])
>> +AC_DEFINE_DIR(SYSCONFDIR, sysconfdir, [sysconfdir])
>>
>> AC_SUBST([RELEASE_DATE])
>> BUILD_DATE="`date +'%Y%m%d'`"
>> @@ -2499,6 +2510,7 @@ dri3/Makefile
>> present/Makefile
>> hw/Makefile
>> hw/xfree86/Makefile
>> +hw/xfree86/Xorg.sh
>> hw/xfree86/common/Makefile
>> hw/xfree86/common/xf86Build.h
>> hw/xfree86/ddc/Makefile
>> diff --git a/hw/xfree86/Makefile.am b/hw/xfree86/Makefile.am
>> index 73e1b4c..a315bbc 100644
>> --- a/hw/xfree86/Makefile.am
>> +++ b/hw/xfree86/Makefile.am
>> @@ -81,9 +81,15 @@ Xorg_DEPENDENCIES = $(LOCAL_LIBS)
>>
>> Xorg_LDFLAGS = $(LD_EXPORT_SYMBOLS_FLAG)
>>
>> +if SUID_WRAPPER
>> +wrapdir = $(SUID_WRAPPER_DIR)
>> +wrap_PROGRAMS = Xorg.wrap
>> +Xorg_wrap_SOURCES = xorg-wrapper.c
>> +endif
>> +
>> BUILT_SOURCES = xorg.conf.example
>> DISTCLEANFILES = xorg.conf.example
>> -EXTRA_DIST = xorgconf.cpp
>> +EXTRA_DIST = xorgconf.cpp Xorg.sh.in
>>
>> # Without logdir, X will post an error on the terminal and will not start
>> install-data-local:
>> @@ -98,6 +104,11 @@ if INSTALL_SETUID
>> chown root $(DESTDIR)$(bindir)/Xorg
>> chmod u+s $(DESTDIR)$(bindir)/Xorg
>> endif
>> +if SUID_WRAPPER
>> + mv $(DESTDIR)$(bindir)/Xorg $(DESTDIR)$(SUID_WRAPPER_DIR)/Xorg.bin
>> + ${INSTALL} -m 755 Xorg.sh $(DESTDIR)$(bindir)/Xorg
>> + -chown root $(DESTDIR)$(SUID_WRAPPER_DIR)/Xorg.wrap && chmod u+s $(DESTDIR)$(SUID_WRAPPER_DIR)/Xorg.wrap
>> +endif
>>
>> uninstall-local:
>> if CYGWIN
>> @@ -119,7 +130,7 @@ xorg.conf.example: xorgconf.cpp
>> relink:
>> $(AM_V_at)rm -f Xorg$(EXEEXT) && $(MAKE) Xorg$(EXEEXT)
>>
>> -CLEANFILES = sdksyms.c sdksyms.dep
>> +CLEANFILES = sdksyms.c sdksyms.dep Xorg.sh
>> EXTRA_DIST += sdksyms.sh
>>
>> sdksyms.dep sdksyms.c: sdksyms.sh
>> diff --git a/hw/xfree86/Xorg.sh.in b/hw/xfree86/Xorg.sh.in
>> new file mode 100644
>> index 0000000..1356e11
>> --- /dev/null
>> +++ b/hw/xfree86/Xorg.sh.in
>> @@ -0,0 +1,12 @@
>> +#!/bin/sh
>> +#
>> +# See if Xorg.wrap is installed and if it is execute it, otherwise execute
>> +# Xorg.bin directly. This allows distros to put the suid wrapper in a
>> +# separate package.
>
> fwiw, this may be better rewritten as "execute Xorg.wrap if it exists,
> otherwise ..."
>
>> +
>> +basedir=@SUID_WRAPPER_DIR@
>> +if [ -x "$basedir"/Xorg.wrap ]; then
>> + exec "$basedir"/Xorg.wrap "$@"
>> +else
>> + exec "$basedir"/Xorg.bin "$@"
>> +fi
>> diff --git a/hw/xfree86/man/Makefile.am b/hw/xfree86/man/Makefile.am
>> index 80e22cb..f41d26c 100644
>> --- a/hw/xfree86/man/Makefile.am
>> +++ b/hw/xfree86/man/Makefile.am
>> @@ -1,3 +1,8 @@
>> include $(top_srcdir)/manpages.am
>> appman_PRE = Xorg.man
>> fileman_PRE = xorg.conf.man xorg.conf.d.man
>> +
>> +if SUID_WRAPPER
>> +appman_PRE += Xorg.wrap.man
>> +fileman_PRE += Xwrapper.config.man
>> +endif
>> diff --git a/hw/xfree86/man/Xorg.wrap.man b/hw/xfree86/man/Xorg.wrap.man
>> new file mode 100644
>> index 0000000..cc62b5b
>> --- /dev/null
>> +++ b/hw/xfree86/man/Xorg.wrap.man
>> @@ -0,0 +1,51 @@
>> +.\" Xwrapper.wrap.1
>> +.\"
>> +.\" Copyright 2014 Red Hat, Inc.
>> +.\"
>> +.\" Permission to use, copy, modify, distribute, and sell this software and its
>> +.\" documentation for any purpose is hereby granted without fee, provided that
>> +.\" the above copyright notice appear in all copies and that both that
>> +.\" copyright notice and this permission notice appear in supporting
>> +.\" documentation.
>> +.\"
>> +.\" The above copyright notice and this permission notice shall be included
>> +.\" in all copies or substantial portions of the Software.
>> +.\"
>> +.\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> +.\" OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +.\" MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
>> +.\" IN NO EVENT SHALL THE OPEN GROUP BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> +.\" OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> +.\" ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> +.\" OTHER DEALINGS IN THE SOFTWARE.
>> +.\"
>> +.\" Except as contained in this notice, the name of The Open Group shall
>> +.\" not be used in advertising or otherwise to promote the sale, use or
>> +.\" other dealings in this Software without prior written authorization
>> +.\" from The Open Group.
>> +.\"
>> +.\" shorthand for double quote that works everywhere.
>> +.ds q \N'34'
>> +.TH Xorg.wrap 1 __xorgversion__
>> +.SH NAME
>> +Xorg.wrap \- Xorg xserver binary wrapper
>
> run a s/xserver/X server/ on the two man pages please, for consistency with Xorg(1)
>
>> +.SH DESCRIPTION
>> +The Xorg xserver may need root rights to function properly. To start the
>> +Xorg xserver with these rights your system is using a suid root wrapper
>> +installed as __suid_wrapper_dir__/Xorg.wrap which will execute the real
>> +xserver which is installed as __suid_wrapper_dir__/Xorg.bin .
>> +.PP
>> +By default Xorg.wrap will autodetect if root rights are necessary, and
>> +if not it will drop its elevated rights before starting the real xserver.
>> +By default Xorg.wrap will only allow executing the real xserver from login
>> +sessions on a physical console. This behavior can be overridden from the
>> +\fI__sysconfdir__/X11/Xwrapper.config\fP config file.
>> +.PP
>> +When auto-detecting the wrapper will drop rights if kms graphics are available
>> +and not drop them if no kms graphics are detected. If a system has multiple
>> +graphics cards and some are not kms capable auto-detection may fail,
>> +in this case manual configuration should be used.
>> +
>> +.SH "SEE ALSO"
>> +Xorg.wrap configuration file: \fIXwrapper.config\fP(5)
>> +Xorg xserver information: \fIXorg\fP(1)
>> diff --git a/hw/xfree86/man/Xwrapper.config.man b/hw/xfree86/man/Xwrapper.config.man
>> new file mode 100644
>> index 0000000..07ecbc7
>> --- /dev/null
>> +++ b/hw/xfree86/man/Xwrapper.config.man
>> @@ -0,0 +1,66 @@
>> +.\" Xwrapper.config.5
>> +.\"
>> +.\" Copyright 2014 Red Hat, Inc.
>> +.\"
>> +.\" Permission to use, copy, modify, distribute, and sell this software and its
>> +.\" documentation for any purpose is hereby granted without fee, provided that
>> +.\" the above copyright notice appear in all copies and that both that
>> +.\" copyright notice and this permission notice appear in supporting
>> +.\" documentation.
>> +.\"
>> +.\" The above copyright notice and this permission notice shall be included
>> +.\" in all copies or substantial portions of the Software.
>> +.\"
>> +.\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> +.\" OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +.\" MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
>> +.\" IN NO EVENT SHALL THE OPEN GROUP BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> +.\" OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> +.\" ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> +.\" OTHER DEALINGS IN THE SOFTWARE.
>> +.\"
>> +.\" Except as contained in this notice, the name of The Open Group shall
>> +.\" not be used in advertising or otherwise to promote the sale, use or
>> +.\" other dealings in this Software without prior written authorization
>> +.\" from The Open Group.
>> +.\"
>> +.\" shorthand for double quote that works everywhere.
>> +.ds q \N'34'
>> +.TH Xwrapper.config 5 __xorgversion__
>> +.SH NAME
>> +Xwrapper.config \- Xorg xserver binary wrapper config file
>> +.SH DESCRIPTION
>> +The Xorg xserver may need root rights to function properly. To start the
>> +Xorg xserver with these rights your system is using a suid root wrapper
>> +installed as __suid_wrapper_dir__/Xorg.wrap which will execute the real
>> +xserver which is installed as __suid_wrapper_dir__/Xorg.bin .
>> +.PP
>> +By default Xorg.wrap will autodetect if root rights are necessary, and
>> +if not it will drop its elevated rights before starting the real xserver.
>> +By default Xorg.wrap will only allow executing the real xserver from login
>> +sessions on a physical console. This behavior can be overridden from the
>> +\fI__sysconfdir__/X11/Xwrapper.config\fP config file.
>
> Given how bad we have been in the past in keeping man pages up-to-date I
> strongly suggest to merge the two pages into one and simply alias it for
> Xwrapper.config(5). The description is already the same anyway.
>
>> +
>> +.SH OPTIONS
>> +Lines starting with a \fB#\fP in Xwrapper.config are considered comments
>> +and will be ignored any other non empty lines must take the form of \fBkey\fP =
>
> missing "." after "ignored".
>
>> +\fIvalue\fP.
>> +.TP 8
>> +\fBallowed_users\fP = \fIrootonly\fP|\fIconsole\fP|\fIanybody\fP
>> +Specify which users may start the xserver through the wrapper. Use
>> +\fIrootonly\fP to only allow root, use \fIconsole\fP to only allow users
>> +logged into a physical console, and use \fIanybody\fP to allow anybody.
>
> "the default is ..."
>
>> +.TP 8
>> +\fBneeds_root_rights\fP = \fIyes\fP|\fIno\fP|\fIauto\fP
>> +Configure if the wrapper should drop its elevated (root) rights before starting
>> +the xserver. Use \fIyes\fP to force execution as root, \fIno\fP to force
>> +execution with all suid rights dropped, and \fIauto\fP to letter the wrapper
>> +auto-detect.
>> +.PP
>> +When auto-detecting the wrapper will drop rights if kms graphics are available
>> +and not drop them if no kms graphics are detected. If a system has multiple
>> +graphics cards and some are not kms capable auto-detection may fail,
>> +in this case manual configuration should be used.
>> +
>> +.SH "SEE ALSO"
>> +Xorg xserver information: \fIXorg\fP(1)
>> diff --git a/hw/xfree86/xorg-wrapper.c b/hw/xfree86/xorg-wrapper.c
>> new file mode 100644
>> index 0000000..221f1b6
>> --- /dev/null
>> +++ b/hw/xfree86/xorg-wrapper.c
>> @@ -0,0 +1,231 @@
>> +/*
>> + * Copyright © 2014 Red Hat, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + * Author: Hans de Goede <hdegoede at redhat.com>
>> + */
>> +
>> +#include "dix-config.h"
>> +
>> +#include <fcntl.h>
>> +#include <limits.h>
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/stat.h>
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +#include <drm/drm.h>
>> +#include <xf86drm.h> /* For DRM_DEV_NAME */
>> +
>> +#define CONFIG_FILE SYSCONFDIR "/X11/Xwrapper.config"
>> +
>> +enum { ROOT_ONLY, CONSOLE_ONLY, ANYBODY };
>> +
>> +/* KISS non locale / LANG parsing isspace version */
>> +static int is_space(char c)
>> +{
>> + return c == ' ' || c == '\t' || c == '\n';
>> +}
>> +
>> +static char *strip(char *s)
>> +{
>> + int i;
>> +
>> + /* Strip leading whitespace */
>> + while (s[0] && is_space(s[0]))
>> + s++;
>> +
>> + /* Strip trailing whitespace */
>> + i = strlen(s) - 1;
>> + while (i >= 0 && is_space(s[i])) {
>> + s[i] = 0;
>> + i--;
>> + }
>> +
>> + return s;
>> +}
>> +
>> +static void parse_config(int *allowed, int *needs_root_rights)
>> +{
>> + FILE *f;
>> + char buf[1024];
>> + char *stripped, *equals, *key, *value;
>> + int line = 0;
>> +
>> + f = fopen(CONFIG_FILE, "r");
>> + if (!f)
>> + return;
>> +
>> + while (fgets(buf, sizeof(buf), f)) {
>> + line++;
>> +
>> + /* Skip comments and empty lines */
>> + stripped = strip(buf);
>> + if (stripped[0] == '#' || stripped[0] == 0)
>> + continue;
>> +
>> + /* Split in a key + value pair */
>> + equals = strchr(stripped, '=');
>> + if (!equals) {
>> + fprintf(stderr, "Syntax error at %s line %d\n", CONFIG_FILE, line);
>> + exit(1);
>> + }
>> + *equals = 0;
>> + key = strip(stripped); /* To remove trailing whitespace from key */
>> + value = strip(equals + 1); /* To remove leading whitespace from val */
>> + if (!key[0]) {
>> + fprintf(stderr, "Missing key at %s line %d\n", CONFIG_FILE, line);
>> + exit(1);
>> + }
>> + if (!value[0]) {
>> + fprintf(stderr, "Missing value at %s line %d\n", CONFIG_FILE, line);
>> + exit(1);
>> + }
>> +
>> + /* And finally process */
>> + if (strcmp(key, "allowed_users") == 0) {
>> + if (strcmp(value, "rootonly") == 0)
>> + *allowed = ROOT_ONLY;
>> + else if (strcmp(value, "console") == 0)
>> + *allowed = CONSOLE_ONLY;
>> + else if (strcmp(value, "anybody") == 0)
>> + *allowed = ANYBODY;
>> + else {
>> + fprintf(stderr,
>> + "Unknown value '%s' for 'allowed_users' at %s line %d\n",
>
> maybe "Invalid" instead of Unknown, since you use that a few lines down.
>
>> + value, CONFIG_FILE, line);
>> + exit(1);
>> + }
>> + }
>> + else if (strcmp(key, "needs_root_rights") == 0) {
>> + if (strcmp(value, "yes") == 0)
>> + *needs_root_rights = 1;
>> + else if (strcmp(value, "no") == 0)
>> + *needs_root_rights = 0;
>> + else if (strcmp(value, "auto") == 0)
>> + *needs_root_rights = -1;
>> + else {
>> + fprintf(stderr,
>> + "Invalid value '%s' for 'needs_root_rights' at %s line %d\n",
>> + value, CONFIG_FILE, line);
>> + exit(1);
>> + }
>> + }
>> + else if (strcmp(key, "nice_value") == 0) {
>> + /* Backward compatibility with older Debian Xwrapper, ignore */
>> + }
>> + else {
>> + fprintf(stderr, "Unknown key '%s' at %s line %d\n", key,
>
> same here, I'd prefer "invalid" over unknown.
>
> for the rest:
> Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
>
Thanks for the review, I've made all suggested changes and added your Reviewed-by
to the commit message.
Regards,
Hans
More information about the xorg-devel
mailing list