[PATCH v2] xfree86: Use xorg.conf.d directory for multiple config files

Dan Nicholson dbn.lists at gmail.com
Wed Dec 16 06:42:56 PST 2009


On Tue, Dec 15, 2009 at 10:18 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Mon, Dec 14, 2009 at 09:09:26PM -0800, Dan Nicholson wrote:
>> Currently there is a single file, xorg.conf, for configuring the server.
>> This works fine most of the time, but it becomes a problem when packages
>> or system services need to adjust the configuration. Instead, allow
>> multiple configuration files to live in a directory. Typically this will
>> be /etc/X11/xorg.conf.d.
>>
>> Files with a suffix of .conf will be read and added to the server
>> configuration after xorg.conf. The server won't fall back to using the
>> auto configuration unless there is no config file and there are no files
>> in the config directory.
>>
>> Right now this uses a simpler search template than the config file
>> search path by not using the command line or environment variable
>> parameters. The matching code was refactored a bit to make this more
>> coherent.
>>
>> Signed-off-by: Dan Nicholson <dbn.lists at gmail.com>
>> ---
>>  Changes since v1:
>>
>>  * scandir is used get the proper ordering of .conf files. This also
>>    makes things cleaner by using ConfigFilter as the filter function
>>    passed to scandir.
>>  * No segfaulting when there is no xorg.conf and xorg.conf.d is empty.
>>    In order to make this work, NULL must be passed back as the directory
>>    to the caller of xf86openConfigFile. When scanning for the directory,
>>    it must have readable .conf files to register.
>>  * Man page documentation.
>>  * A separate template is provided for the directory searching instead
>>    of overloading the config file searching.
>>  * More refactoring to avoid forced line breaks.
>>
>>  configure.ac                         |    2 +
>>  cpprules.in                          |    3 +-
>>  hw/xfree86/common/xf86Config.c       |   37 +++--
>>  hw/xfree86/doc/man/xorg.conf.man.pre |   55 +++++--
>>  hw/xfree86/parser/scan.c             |  308 ++++++++++++++++++++++++----------
>>  hw/xfree86/parser/xf86Parser.h       |   12 +-
>>  hw/xwin/winconfig.c                  |   43 ++++-
>>  include/xorg-config.h.in             |    3 +
>>  8 files changed, 340 insertions(+), 123 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 6cdef15..950dee0 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -1720,6 +1720,7 @@ if test "x$XORG" = xyes; then
>>
>>       dnl these only go in xorg-config.h
>>       XF86CONFIGFILE="xorg.conf"
>> +     XF86CONFIGDIR="xorg.conf.d"
>>       CONFIGFILE="$sysconfdir/$XF86CONFIGFILE"
>>       LOGPREFIX="$logdir/Xorg."
>>       AC_DEFINE(XORG_SERVER, 1, [Building Xorg server])
>> @@ -1732,6 +1733,7 @@ if test "x$XORG" = xyes; then
>>       AC_DEFINE(__XSERVERNAME__, "Xorg", [Name of X server])
>>       AC_DEFINE_DIR(__XCONFIGFILE__, XF86CONFIGFILE, [Name of configuration file])
>>       AC_DEFINE_DIR(XF86CONFIGFILE, XF86CONFIGFILE, [Name of configuration file])
>> +     AC_DEFINE_DIR(__XCONFIGDIR__, XF86CONFIGDIR, [Name of configuration directory])
>
> nitpick: moduledir and libdir are both directories as well and and referred
> to with "path", not with "name". Maybe this should be used here to? (e.g.
> "Configuration directory path")

I originally had path, but that's not correct. This define (and
__XCONFIGFILE__) only tell you what the basename is. I.e., xorg.conf
or xorg.conf.d. moduledir is a path (/usr/lib/xorg/modules). I can
change it if it's a big deal, but I think this is right.

>>       AC_DEFINE_DIR(DEFAULT_MODULE_PATH, moduledir, [Default module search path])
>>       AC_DEFINE_DIR(DEFAULT_LIBRARY_PATH, libdir, [Default library install path])
>>       AC_DEFINE_DIR(DEFAULT_LOGPREFIX, LOGPREFIX, [Default log location])
>> diff --git a/cpprules.in b/cpprules.in
>> index 7fcb9bd..7219e36 100644
>> --- a/cpprules.in
>> +++ b/cpprules.in
>> @@ -36,7 +36,8 @@ MANDEFS = \
>>       -D__adminmansuffix__=$(ADMIN_MAN_SUFFIX) \
>>       -D__mandir__=$(mandir) \
>>       -D__projectroot__=$(prefix) \
>> -     -D__xconfigfile__=$(__XCONFIGFILE__) -D__xconfigdir__=$(XCONFIGDIR) \
>> +     -D__xconfigfile__=$(__XCONFIGFILE__) \
>> +     -D__xconfigdir__=$(__XCONFIGDIR__) \
>>       -D__xkbdir__=$(XKB_BASE_DIRECTORY) \
>>       -D__modulepath__="$(DEFAULT_MODULE_PATH)" \
>>       -D__xlogfile__=$(XLOGFILE) -D__xservername__=$(XSERVERNAME)
>> diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
>> index e1283f9..df70950 100644
>> --- a/hw/xfree86/common/xf86Config.c
>> +++ b/hw/xfree86/common/xf86Config.c
>> @@ -102,6 +102,13 @@ extern DeviceAssocRec mouse_assoc;
>>                       "%P/lib/X11/%X.%H," "%P/lib/X11/%X-%M," \
>>                       "%P/lib/X11/%X"
>>  #endif
>> +#ifndef DIR_CONFIGPATH
>> +#define DIR_CONFIGPATH       "/etc/X11/%X-%M," "/etc/X11/%X," "/etc/%X," \
>> +                     "%P/etc/X11/%X.%H," "%P/etc/X11/%X-%M," \
>> +                     "%P/etc/X11/%X," \
>> +                     "%P/lib/X11/%X.%H," "%P/lib/X11/%X-%M," \
>> +                     "%P/lib/X11/%X"
>> +#endif
>>  #ifndef PROJECTROOT
>>  #define PROJECTROOT  "/usr/X11R6"
>>  #endif
>> @@ -2437,7 +2444,7 @@ checkInput(serverLayoutPtr layout, Bool implicit_layout) {
>>  ConfigStatus
>>  xf86HandleConfigFile(Bool autoconfig)
>>  {
>> -    const char *filename;
>> +    XF86ConfPathsPtr paths;
>>      char *searchpath;
>>      MessageType from = X_DEFAULT;
>>      char *scanptr;
>> @@ -2453,18 +2460,26 @@ xf86HandleConfigFile(Bool autoconfig)
>>       if (xf86ConfigFile)
>>           from = X_CMDLINE;
>>
>> -     filename = xf86openConfigFile(searchpath, xf86ConfigFile, PROJECTROOT);
>> -     if (filename) {
>> -         xf86MsgVerb(from, 0, "Using config file: \"%s\"\n", filename);
>> -         xf86ConfigFile = xnfstrdup(filename);
>> -     } else {
>> -         if (xf86ConfigFile)
>> -             xf86Msg(X_ERROR, "Unable to locate/open config file: \"%s\"\n",
>> -                     xf86ConfigFile);
>> +     paths = xf86openConfigFile(searchpath, DIR_CONFIGPATH,
>> +                                xf86ConfigFile, PROJECTROOT);
>> +     if (paths && (paths->file || paths->dir)) {
>> +         if (paths->file) {
>> +             xf86MsgVerb(from, 0, "Using config file: \"%s\"\n",
>> +                         paths->file);
>> +             xf86ConfigFile = xnfstrdup(paths->file);
>> +         } else {
>> +             if (xf86ConfigFile)
>> +                 xf86Msg(X_ERROR,
>> +                         "Unable to locate/open config file: \"%s\"\n",
>> +                         xf86ConfigFile);
>> +         }
>> +         if (paths && paths->dir)
>> +             xf86MsgVerb(X_DEFAULT, 0, "Using config directory: \"%s\"\n",
>> +                         paths->dir);
>> +     } else
>>           return CONFIG_NOFILE;
>> -     }
>>      }
>> -
>> +
>>      if ((xf86configptr = xf86readConfigFile ()) == NULL) {
>>       xf86Msg(X_ERROR, "Problem parsing the config file\n");
>>       return CONFIG_PARSE_ERROR;
>> diff --git a/hw/xfree86/doc/man/xorg.conf.man.pre b/hw/xfree86/doc/man/xorg.conf.man.pre
>> index ace041c..d5260d7 100644
>> --- a/hw/xfree86/doc/man/xorg.conf.man.pre
>> +++ b/hw/xfree86/doc/man/xorg.conf.man.pre
>> @@ -2,27 +2,35 @@
>>  .ds q \N'34'
>>  .TH __xconfigfile__ __filemansuffix__ __vendorversion__
>>  .SH NAME
>> -__xconfigfile__ \- configuration File for __xservername__ X server
>> +__xconfigfile__ and __xconfigdir__ \- configuration files for
>> +__xservername__ X server
>>  .SH INTRODUCTION
>>  .B __xservername__
>>  supports several mechanisms for supplying/obtaining configuration and
>>  run-time parameters: command line options, environment variables, the
>> -__xconfigfile__ configuration file, auto-detection, and fallback defaults.
>> -When the same information is supplied in more than one way, the highest
>> -precedence mechanism is used.  The list of mechanisms is ordered from
>> -highest precedence to lowest.  Note that not all parameters can be
>> -supplied via all methods.  The available command line options and
>> -environment variables (and some defaults) are described in the Xserver(__appmansuffix__)
>> -and __xservername__(__appmansuffix__) manual pages.  Most configuration file parameters, with
>> -their defaults, are described below.  Driver and module specific
>> -configuration parameters are described in the relevant driver or module
>> -manual page.
>> +__xconfigfile__ and __xconfigdir__ configuration files, auto-detection,
>> +and fallback defaults. When the same information is supplied in more
>> +than one way, the highest precedence mechanism is used. The list of
>> +mechanisms is ordered from highest precedence to lowest. Note that not
>> +all parameters can be supplied via all methods. The available command
>> +line options and environment variables (and some defaults) are
>> +described in the Xserver(__appmansuffix__) and
>> +__xservername__(__appmansuffix__) manual pages. Most configuration file
>> +parameters, with their defaults, are described below. Driver and module
>> +specific configuration parameters are described in the relevant driver
>> +or module manual page.
>>  .SH DESCRIPTION
>>  .B __xservername__
>> -uses a configuration file called
>> +uses configuration file called
>
>       ^^ typo?

Good catch.

>>  .I __xconfigfile__
>> +and files ending in the suffix
>> +.I .conf
>> +from the directory
>> +.I __xconfigdir__
>>  for its initial setup.
>> -This configuration file is searched for in the following places when the
>> +The
>> +.I __xconfigfile__
>> +configuration file is searched for in the following places when the
>>  server is started as a normal user:
>>  .PP
>>  .RS 4
>> @@ -93,9 +101,28 @@ directory), and
>>  is the machine's hostname as reported by
>>  .BR gethostname (__libmansuffix__).
>>  .PP
>> +Additional configuration files are searched for in the following
>> +directories:
>> +.PP
>> +.RS 4
>> +.nf
>> +.I /etc/X11/__xconfigdir__\-4
>> +.I /etc/X11/__xconfigdir__
>> +.I /etc/__xconfigdir__
>> +.IR __projectroot__/etc/X11/__xconfigdir__. <hostname>
>> +.I __projectroot__/etc/X11/__xconfigdir__\-4
>> +.I __projectroot__/etc/X11/__xconfigdir__
>> +.IR __projectroot__/lib/X11/__xconfigdir__. <hostname>
>> +.I __projectroot__/lib/X11/__xconfigdir__\-4
>> +.I __projectroot__/lib/X11/__xconfigdir__
>> +.fi
>> +.RE
>> +.PP
>>  The
>>  .I __xconfigfile__
>> -file is composed of a number of sections which may be present in any order,
>> +and
>> +.I __xconfigdir__
>> +files are composed of a number of sections which may be present in any order,
>>  or omitted to use default configuration values.
>>  Each section has the form:
>>  .PP
>> diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
>> index d2e8b6d..1f08263 100644
>> --- a/hw/xfree86/parser/scan.c
>> +++ b/hw/xfree86/parser/scan.c
>> @@ -62,8 +62,11 @@
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <string.h>
>> +#include <sys/types.h>
>> +#include <dirent.h>
>>  #include <unistd.h>
>>  #include <stdarg.h>
>> +#include <X11/Xdefs.h>
>>  #include <X11/Xfuncproto.h>
>>
>>  #if defined(_POSIX_SOURCE)
>> @@ -90,17 +93,23 @@
>>  #include "xf86tokens.h"
>>
>>  #define CONFIG_BUF_LEN     1024
>> +#define CONFIG_MAX_FILES   64
>>
>>  static int StringToToken (char *, xf86ConfigSymTabRec *);
>>
>> -static FILE *configFile = NULL;
>> +static struct {
>> +     FILE *file;
>> +     char *path;
>> +} configFiles[CONFIG_MAX_FILES];
>>  static const char **builtinConfig = NULL;
>>  static int builtinIndex = 0;
>>  static int configPos = 0;            /* current readers position */
>>  static int configLineNo = 0; /* linenumber */
>>  static char *configBuf, *configRBuf; /* buffer for lines */
>> -static char *configPath;             /* path to config file */
>> +static XF86ConfPathsRec configPaths = { NULL };      /* paths to configuration */
>>  static char *configSection = NULL;   /* name of current section being parsed */
>> +static int numFiles = 0;             /* number of config files */
>> +static int curFileIndex = 0;         /* index of current config file */
>>  static int pushToken = LOCK_TOKEN;
>>  static int eol_seen = 0;             /* private state to handle comments */
>>  LexRec val;
>> @@ -155,7 +164,7 @@ xf86strToUL (char *str)
>>  /*
>>   * xf86getNextLine --
>>   *
>> - *  read from the configFile FILE stream until we encounter a new
>> + *  read from the configFiles FILE stream until we encounter a new
>>   *  line; this is effectively just a big wrapper for fgets(3).
>>   *
>>   *  xf86getToken() assumes that we will read up to the next
>> @@ -213,9 +222,18 @@ xf86getNextLine(void)
>>       /* read in another block of chars */
>>
>>       do {
>> -             ret = fgets(configBuf + pos, configBufLen - pos - 1, configFile);
>> +             ret = fgets(configBuf + pos, configBufLen - pos - 1,
>> +                         configFiles[curFileIndex].file);
>>
>> -             if (!ret) break;
>> +             if (!ret) {
>> +                     /* stop if there are no more files */
>> +                     if (++curFileIndex >= numFiles) {
>> +                             curFileIndex = 0;
>> +                             break;
>> +                     }
>> +                     configLineNo = 0;
>> +                     continue;
>> +             }
>>
>>               /* search for EOL in the new block of chars */
>>
>> @@ -306,7 +324,7 @@ again:
>>               if (!c)
>>               {
>>                       char *ret;
>> -                     if (configFile)
>> +                     if (numFiles > 0)
>>                               ret = xf86getNextLine();
>>                       else {
>>                               if (builtinConfig[builtinIndex] == NULL)
>> @@ -575,6 +593,12 @@ xf86pathIsSafe(const char *path)
>>  #ifndef XCONFIGFILE
>>  #define XCONFIGFILE  "xorg.conf"
>>  #endif
>> +#ifndef XCONFIGDIR
>> +#define XCONFIGDIR   "xorg.conf.d"
>> +#endif
>> +#ifndef XCONFIGSUFFIX
>> +#define XCONFIGSUFFIX        ".conf"
>> +#endif
>>  #ifndef PROJECTROOT
>>  #define PROJECTROOT  "/usr/X11R6"
>>  #endif
>> @@ -616,7 +640,8 @@ xf86pathIsSafe(const char *path)
>>
>>  static char *
>>  DoSubstitution(const char *template, const char *cmdline, const char *projroot,
>> -                             int *cmdlineUsed, int *envUsed, char *XConfigFile)
>> +                             int *cmdlineUsed, int *envUsed,
>> +                             const char *XConfigFile)
>>  {
>>       char *result;
>>       int i, l;
>> @@ -745,20 +770,152 @@ DoSubstitution(const char *template, const char *cmdline, const char *projroot,
>>       return result;
>>  }
>>
>> -/*
>> +/*
>> + * Given some searching parameters, locate and open the xorg config file.
>> + */
>> +static char *
>> +OpenConfigFile(const char *path, const char *cmdline, const char *projroot,
>> +            const char *confname)
>> +{
>> +     char *filepath = NULL;
>> +     char *pathcopy;
>> +     const char *template;
>> +     int cmdlineUsed = 0;
>> +     FILE *file = NULL;
>> +
>> +     pathcopy = strdup(path);
>> +     for (template = strtok(pathcopy, ","); template && !file;
>> +          template = strtok(NULL, ",")) {
>> +             filepath = DoSubstitution(template, cmdline, projroot,
>> +                                       &cmdlineUsed, NULL, confname);
>> +             if (!filepath)
>> +                     continue;
>> +             if (cmdline && !cmdlineUsed) {
>> +                     free(filepath);
>> +                     filepath = NULL;
>> +                     continue;
>> +             }
>> +             file = fopen(filepath, "r");
>> +             if (!file) {
>> +                     free(filepath);
>> +                     filepath = NULL;
>> +             }
>> +     }
>> +
>> +     if (file) {
>> +             configFiles[numFiles].file = file;
>> +             configFiles[numFiles].path = strdup(filepath);
>> +             numFiles++;
>> +     }
>> +     return filepath;
>> +}
>> +
>> +/*
>> + * Match non-hidden files in the xorg config directory with a .conf
>> + * suffix. This filter is passed to scandir(3).
>> + */
>> +static int
>> +ConfigFilter(const struct dirent *de)
>> +{
>> +     const char *name = de->d_name;
>> +     size_t len = strlen(name);
>> +     size_t suflen = strlen(XCONFIGSUFFIX);
>> +
>> +     if (!name || name[0] == '.' || len <= suflen)
>> +             return 0;
>> +     if (strcmp(&name[len-suflen], XCONFIGSUFFIX) != 0)
>> +             return 0;
>> +     return 1;
>> +}
>> +
>> +static Bool
>> +AddConfigDirFiles(const char *dirpath, struct dirent **list, int num)
>> +{
>> +     int i;
>> +     Bool openedFile = FALSE;
>> +     Bool warnOnce = FALSE;
>> +
>> +     for (i = 0; i < num; i++) {
>> +             char *path;
>> +             FILE *file;
>> +
>> +             if (numFiles >= CONFIG_MAX_FILES) {
>> +                     if (!warnOnce)
>> +                             ErrorF("Maximum number of configuration "
>> +                                    "files opened\n");
>
> warnOnce should be set here, right?

Good catch. This was a late addition...


>> +                     free(list[i]);
>> +                     continue;
>> +             }
>> +
>> +             path = malloc(PATH_MAX + 1);
>
> unrelated, but - malloc? xalloc? we have both of them in use already though
> xalloc hold the majority for now. eventually we should just pick one. same
> for free/xfree.

Ha. I think I used malloc because the original code did. I can just
make it use xalloc/xfree until some time that it's decided that the
wrappers suck.


>> +             snprintf(path, PATH_MAX + 1, "%s/%s", dirpath,
>> +                      list[i]->d_name);
>> +             free(list[i]);
>> +             file = fopen(path, "r");
>> +             if (!file) {
>> +                     free(path);
>> +                     continue;
>> +             }
>> +             openedFile = TRUE;
>> +
>> +             configFiles[numFiles].file = file;
>> +             configFiles[numFiles].path = path;
>> +             numFiles++;
>> +     }
>> +
>> +     return openedFile;
>> +}
>> +
>> +/*
>> + * Given some searching parameters, locate and open the xorg config
>> + * directory. The directory does not need to contain config files.
>> + */
>> +static char *
>> +OpenConfigDir(const char *path, const char *projroot, const char *confname)
>> +{
>> +     char *dirpath, *pathcopy;
>> +     const char *template;
>> +     Bool found = FALSE;
>> +
>> +     pathcopy = strdup(path);
>> +     for (template = strtok(pathcopy, ","); template && !found;
>> +          template = strtok(NULL, ",")) {
>> +             struct dirent **list = NULL;
>> +             int num;
>> +
>> +             if (!(dirpath = DoSubstitution(template, NULL, projroot,
>> +                                            NULL, NULL, confname)))
>> +                     continue;
>> +             /* match files named *.conf */
>> +             num = scandir(dirpath, &list, ConfigFilter, alphasort);
>> +             found = AddConfigDirFiles(dirpath, list, num);
>> +             if (!found) {
>> +                     free(dirpath);
>> +                     dirpath = NULL;
>> +                     if (list)
>> +                             free(list);
>> +             }
>> +     }
>> +
>> +     return dirpath;
>> +}
>> +
>> +/*
>>   * xf86openConfigFile --
>>   *
>> - * This function take a config file search path (optional), a command-line
>> - * specified file name (optional) and the ProjectRoot path (optional) and
>> - * locates and opens a config file based on that information.  If a
>> + * This function take a config file search path (optional), a config
>> + * directory search path (optional), command-line specified file name
>> + * (optional) and the ProjectRoot path (optional) and locates and opens
>> + * a config file and config directory based on that information.  If a
>>   * command-line file name is specified, then this function fails if none
>>   * of the located files.
>>   *
>> - * The return value is a pointer to the actual name of the file that was
>> - * opened.  When no file is found, the return value is NULL.
>> + * The return value is a pointer to a structure with the the actual name of
>> + * the file and directory that were opened.  The entries will be NULL if
>> + * they are not found.
>>   *
>>   * The escape sequences allowed in the search path are defined above.
>> - *
>> + *
>>   */
>>
>>  #ifndef DEFAULT_CONF_PATH
>> @@ -776,108 +933,85 @@ DoSubstitution(const char *template, const char *cmdline, const char *projroot,
>>                                                       "%P/lib/X11/%X-%M," \
>>                                                       "%P/lib/X11/%X"
>>  #endif
>> +#ifndef DEFAULT_DIR_PATH
>> +#define DEFAULT_DIR_PATH     "/etc/X11/%S," \
>> +                             "%P/etc/X11/%S," \
>> +                             "/etc/X11/%X-%M," \
>> +                             "/etc/X11/%X," \
>> +                             "/etc/%X," \
>> +                             "%P/etc/X11/%X.%H," \
>> +                             "%P/etc/X11/%X-%M," \
>> +                             "%P/etc/X11/%X," \
>> +                             "%P/lib/X11/%X.%H," \
>> +                             "%P/lib/X11/%X-%M," \
>> +                             "%P/lib/X11/%X"
>> +#endif
>>
>> -const char *
>> -xf86openConfigFile(const char *path, const char *cmdline, const char *projroot)
>> +const XF86ConfPathsPtr
>> +xf86openConfigFile(const char *filepath, const char *dirpath,
>> +                const char *cmdline, const char *projroot)
>>  {
>> -     char *pathcopy;
>> -     const char *template;
>> -     int cmdlineUsed = 0;
>> -
>> -     configFile = NULL;
>> +     memset(configFiles, 0, sizeof(configFiles));
>> +     numFiles = 0;
>> +     curFileIndex = 0;
>>       configPos = 0;          /* current readers position */
>>       configLineNo = 0;       /* linenumber */
>>       pushToken = LOCK_TOKEN;
>>
>> -     if (!path || !path[0])
>> -             path = DEFAULT_CONF_PATH;
>> -     pathcopy = malloc(strlen(path) + 1);
>> -     strcpy(pathcopy, path);
>> +     if (!filepath || !filepath[0])
>> +             filepath = DEFAULT_CONF_PATH;
>> +     if (!dirpath || !dirpath[0])
>> +             dirpath = DEFAULT_DIR_PATH;
>>       if (!projroot || !projroot[0])
>>               projroot = PROJECTROOT;
>>
>> -     template = strtok(pathcopy, ",");
>> -
>> -     /* First, search for a config file. */
>> -     while (template && !configFile) {
>> -             if ((configPath = DoSubstitution(template, cmdline, projroot,
>> -                                              &cmdlineUsed, NULL,
>> -                                              XCONFIGFILE))) {
>> -                     if ((configFile = fopen(configPath, "r")) != 0) {
>> -                             if (cmdline && !cmdlineUsed) {
>> -                                     fclose(configFile);
>> -                                     configFile = NULL;
>> -                             }
>> -                     }
>> -             }
>> -             if (configPath && !configFile) {
>> -                     free(configPath);
>> -                     configPath = NULL;
>> -             }
>> -             template = strtok(NULL, ",");
>> -     }
>> -
>> -     /* Then search for fallback */
>> -     if (!configFile) {
>> -         strcpy(pathcopy, path);
>> -         template = strtok(pathcopy, ",");
>> -
>> -         while (template && !configFile) {
>> -             if ((configPath = DoSubstitution(template, cmdline, projroot,
>> -                                              &cmdlineUsed, NULL,
>> -                                              XFREE86CFGFILE))) {
>> -                 if ((configFile = fopen(configPath, "r")) != 0) {
>> -                     if (cmdline && !cmdlineUsed) {
>> -                         fclose(configFile);
>> -                         configFile = NULL;
>> -                     }
>> -                 }
>> -             }
>> -             if (configPath && !configFile) {
>> -                 free(configPath);
>> -                 configPath = NULL;
>> -             }
>> -             template = strtok(NULL, ",");
>> -         }
>> -     }
>> -
>> -     free(pathcopy);
>> -     if (!configFile) {
>> -
>> -             return NULL;
>> -     }
>> +     /* Search for a config file or a fallback */
>> +     configPaths.file = OpenConfigFile(filepath, cmdline, projroot,
>> +                                       XCONFIGFILE);
>> +     if (!configPaths.file)
>> +             configPaths.file = OpenConfigFile(filepath, cmdline, projroot,
>> +                                               XFREE86CFGFILE);
>> +     /* Search for the multiconf directory */
>> +     configPaths.dir = OpenConfigDir(dirpath, projroot, XCONFIGDIR);
>>
>>       configBuf = malloc (CONFIG_BUF_LEN);
>>       configRBuf = malloc (CONFIG_BUF_LEN);
>>       configBuf[0] = '\0';            /* sanity ... */
>>
>> -     return configPath;
>> +     return &configPaths;
>>  }
>>
>>  void
>>  xf86closeConfigFile (void)
>>  {
>> -     free (configPath);
>> -     configPath = NULL;
>> +     int i;
>> +
>> +     free (configPaths.file);
>> +     configPaths.file = NULL;
>> +     free (configPaths.dir);
>> +     configPaths.dir = NULL;
>>       free (configRBuf);
>>       configRBuf = NULL;
>>       free (configBuf);
>>       configBuf = NULL;
>>
>> -     if (configFile) {
>> -             fclose (configFile);
>> -             configFile = NULL;
>> -     } else {
>> +     if (numFiles == 0) {
>>               builtinConfig = NULL;
>>               builtinIndex = 0;
>>       }
>> +     for (i = 0; i < numFiles; i++) {
>> +             fclose(configFiles[i].file);
>> +             configFiles[i].file = NULL;
>> +             free(configFiles[i].path);
>> +             configFiles[i].path = NULL;
>> +     }
>> +     numFiles = 0;
>>  }
>>
>>  void
>>  xf86setBuiltinConfig(const char *config[])
>>  {
>>       builtinConfig = config;
>> -     configPath = strdup("<builtin configuration>");
>>       configBuf = malloc (CONFIG_BUF_LEN);
>>       configRBuf = malloc (CONFIG_BUF_LEN);
>>       configBuf[0] = '\0';            /* sanity ... */
>> @@ -888,9 +1022,11 @@ void
>>  xf86parseError (char *format,...)
>>  {
>>       va_list ap;
>> +     char *filename = numFiles ? configFiles[curFileIndex].path :
>> +                      "<builtin configuration>";
>>
>>       ErrorF ("Parse error on line %d of section %s in file %s\n\t",
>> -              configLineNo, configSection, configPath);
>> +              configLineNo, configSection, filename);
>>       va_start (ap, format);
>>       VErrorF (format, ap);
>>       va_end (ap);
>> @@ -902,8 +1038,10 @@ void
>>  xf86validationError (char *format,...)
>>  {
>>       va_list ap;
>> +     char *filename = numFiles ? configFiles[curFileIndex].path :
>> +                      "<builtin configuration>";
>>
>> -     ErrorF ("Data incomplete in file %s\n\t", configPath);
>> +     ErrorF ("Data incomplete in file %s\n\t", filename);
>>       va_start (ap, format);
>>       VErrorF (format, ap);
>>       va_end (ap);
>> diff --git a/hw/xfree86/parser/xf86Parser.h b/hw/xfree86/parser/xf86Parser.h
>> index 6030800..3d9bece 100644
>> --- a/hw/xfree86/parser/xf86Parser.h
>> +++ b/hw/xfree86/parser/xf86Parser.h
>> @@ -453,11 +453,19 @@ typedef struct
>>  }
>>  xf86ConfigSymTabRec, *xf86ConfigSymTabPtr;
>>
>> +typedef struct
>> +{
>> +     char *file;     /* config file */
>> +     char *dir;      /* multiconf directory */
>> +}
>> +XF86ConfPathsRec, *XF86ConfPathsPtr;
>> +
>>  /*
>>   * prototypes for public functions
>>   */
>> -extern _X_EXPORT const char *xf86openConfigFile (const char *, const char *,
>> -                                     const char *);
>> +extern _X_EXPORT const XF86ConfPathsPtr
>> +xf86openConfigFile(const char *filepath, const char *dirpath,
>> +                const char *cmdline, const char *projroot);
>
>
> s/_X_EXPORT// should work here.

Yeah, I forgot about that, but all the ones below this should be, too.
I think it would be better to unexport all these functions in a
separate patch.

>>  extern _X_EXPORT void xf86setBuiltinConfig(const char *config[]);
>>  extern _X_EXPORT XF86ConfigPtr xf86readConfigFile (void);
>>  extern _X_EXPORT void xf86closeConfigFile (void);
>> diff --git a/hw/xwin/winconfig.c b/hw/xwin/winconfig.c
>> index 3e1908c..51981c4 100644
>> --- a/hw/xwin/winconfig.c
>> +++ b/hw/xwin/winconfig.c
>> @@ -50,6 +50,13 @@
>>                      "%P/lib/X11/%X.%H," "%P/lib/X11/%X-%M," \
>>                      "%P/lib/X11/%X"
>>  #endif
>> +#ifndef CONFIGDIRPATH
>> +#define CONFIGDIRPATH  "/etc/X11/%X-%M," "/etc/X11/%X," "/etc/%X," \
>> +                       "%P/etc/X11/%X.%H," "%P/etc/X11/%X-%M," \
>> +                       "%P/etc/X11/%X," \
>> +                       "%P/lib/X11/%X.%H," "%P/lib/X11/%X-%M," \
>> +                       "%P/lib/X11/%X"
>> +#endif
>>
>>  XF86ConfigPtr g_xf86configptr = NULL;
>>  #endif
>> @@ -109,7 +116,9 @@ Bool
>>  winReadConfigfile ()
>>  {
>>    Bool               retval = TRUE;
>> +  XF86ConfPathsPtr   paths;
>>    const char *filename;
>> +  const char *dirname;
>>    MessageType        from = X_DEFAULT;
>>    char               *xf86ConfigFile = NULL;
>>
>> @@ -121,24 +130,38 @@ winReadConfigfile ()
>>
>>    /* Parse config file into data structure */
>>
>> -  filename = xf86openConfigFile (CONFIGPATH, xf86ConfigFile, PROJECTROOT);
>> -
>> +  paths = xf86openConfigFile (CONFIGPATH, CONFIGDIRPATH, xf86ConfigFile,
>> +                           PROJECTROOT);
>> +
>>    /* Hack for backward compatibility */
>> -  if (!filename && from == X_DEFAULT)
>> -    filename = xf86openConfigFile (CONFIGPATH, "XF86Config", PROJECTROOT);
>> +  if (!(paths && paths->file) && from == X_DEFAULT)
>> +    paths = xf86openConfigFile (CONFIGPATH, CONFIGDIRPATH, "XF86Config",
>> +                             PROJECTROOT);
>>
>> -  if (filename)
>> +  if (paths && (paths->file || paths->dir))
>>      {
>> -      winMsg (from, "Using config file: \"%s\"\n", filename);
>> +      if (paths && paths->file)
>> +     {
>> +       winMsg (from, "Using config file: \"%s\"\n", paths->file);
>> +     }
>> +      else
>> +     {
>> +       winMsg (X_ERROR, "Unable to locate/open config file");
>> +       if (xf86ConfigFile)
>> +         ErrorF (": \"%s\"", xf86ConfigFile);
>> +       ErrorF ("\n");
>> +     }
>> +
>> +      if (paths && paths->dir)
>> +     {
>> +       winMsg (X_DEFAULT, "Using config directory \"%s\"\n", paths->dir);
>> +     }
>>      }
>>    else
>>      {
>> -      winMsg (X_ERROR, "Unable to locate/open config file");
>> -      if (xf86ConfigFile)
>> -     ErrorF (": \"%s\"", xf86ConfigFile);
>> -      ErrorF ("\n");
>>        return FALSE;
>>      }
>> +
>>    if ((g_xf86configptr = xf86readConfigFile ()) == NULL)
>>      {
>>        winMsg (X_ERROR, "Problem parsing the config file\n");
>> diff --git a/include/xorg-config.h.in b/include/xorg-config.h.in
>> index 794de7a..d28dc0d 100644
>> --- a/include/xorg-config.h.in
>> +++ b/include/xorg-config.h.in
>> @@ -36,6 +36,9 @@
>>  /* Path to configuration file. */
>>  #undef __XCONFIGFILE__
>>
>> +/* Name of configuration directory. */
>> +#undef __XCONFIGDIR__
>> +
>>  /* Path to loadable modules. */
>>  #undef DEFAULT_MODULE_PATH
>>
>> --
>> 1.6.2.5
>
> tested, works. I'd be happy to see this patch with these last few changes
> merged.

Strange to say this since I just submitted this patch, but I think it
might be better to make xf86openConfigFile two functions in light
Daniel and Alan's discussion about having separate command lines for
config file and config dir. xf86openConfigFile is a little overloaded
now. I can refactor that now, or we can wait until that discussion
gets more concrete. I didn't put a lot of thought into this until
after I sent this.

--
Dan


More information about the xorg-devel mailing list