intel driver will only compile with gcc

[Kean Johnston]

> The section of the code that defines the default set of modules to be
> loaded uses named initializers. See commit
> e91b9ddc7aa95abc2d4d314e8db204860771a099. The rationale for this is that it
> makes it very easy to change the default set to whatever the user wants. 
> Named initializers are very nice, and I'd personally like to keep them.
The X11 codebase has always been a bastion of extreme portability. There
have been countless times over the course of its life where code has been
written in a way that may not be as pretty as a given alternative, because
it is portable. The case you cite is a perfect example of that. The code
you reference has:

typedef struct _ModuleDefault {
     char *name;
     Bool toLoad;
     XF86OptionPtr load_opt;
} ModuleDefault;

static ModuleDefault ModuleDefaults[] = {
     {.name = "extmod",   .toLoad = TRUE,    .load_opt=NULL},
     {.name = "dbe",      .toLoad = TRUE,    .load_opt=NULL},
  ...
};

That code works only with a C99 compiler. You can achieve the exact same
thing, just as legibly, and just as easily to change which drivers are
loaded, by having the initializer read:


static ModuleDefault ModuleDefaults[] = {
     {"extmod",   TRUE,    NULL},
     {"dbe",      TRUE,    NULL},
  ...
};

The named initializer buys you absolutely nothing whatsoever. But it costs
you the ability to compile using compilers that X11 has supported for
decades.

The intel driver still only works with GCC, because even if you keep that
section allowing variadic macros (which I really hope we can drop), it
doesn't even use the macros in the C99 way, but rather in the pre-C99
gcc-sepcific way. To wit:

#define I830FALLBACK(s, arg...)                        \
do {                                                   \
        DPRINTF(PFX, "EXA fallback: " s "\n", ##arg);   \
        return FALSE;                                   \
} while(0)

This is non-portable and only works with gcc, whereas:
#define I830FALLBACK(s, ...)                                 \
do {                                                         \
      DPRINTF(PFX, "EXA fallback: " s "\n", __VA_ARGS__);     \
      return FALSE;                                           \
} while(0)

actually follows the C99 standard. Yes, it means that any place where
you use the macro with no arguments you need to have a trailing comma:

current gcc-only way:
        I830FALLBACK("Get Color buffer format\n");

portable C99 way:
        I830FALLBACK("Get Color buffer format\n",);

The alternative, which works on just about every C compiler ever
written, is to have 2 or 3 macros:

#define I830FALLBACK(s)                         \
do {                                            \
        DPRINTF(PFX, "EXA fallback: " s "\n";    \
        return FALSE;                            \
} while(0)

#define I830FALLBACK1(s,arg1)                         \
do {                                                  \
        DPRINTF(PFX, "EXA fallback: " s "\n", arg1);   \
        return FALSE;                                  \
} while(0)

#define I830FALLBACK2(s,arg1,arg2)                      \
do {                                                    \
        DPRINTF(PFX, "EXA fallback: " s "\n", arg1,arg2; \
        return FALSE;                                    \
} while(0)


I will freely admit its not as "pretty" or "elegant". I guess the X.org
team needs to decide which is more important to them: pretty code or
code that works for a significantly larger community, a large part of
which is silent and won't ever complain directly to X.org, but instead
to systems integrators who themselves frequently just silently patch
around this sort of shenanigans.

I am hoping hat Keith and/or Daniel are reading this thread and can be
persuaded to please drop that section of the C extensions document to help
better serve the community.

Kean