[PATCH 1/2] dix: make screens structures run-time adjustable

Jamey Sharp jamey at minilop.net
Mon Apr 12 09:32:24 PDT 2010


I'd rather have this patch than keep MAXSCREENS. :-) But I think it
could be better...

I notice that every call to MAXSCREENSALLOC and friends is passed a
size of screenInfo.numScreens. That suggests these macros are
misnamed, because they're really generic xalloc wrappers that do a
multiplication for you. (They don't even check for overflow, which
probably doesn't matter here, but it's the only reason I'd want to
have an API for doing multiplication...)

Do we really need to introduce a new pile of magic macros? If I were
going to introduce a macro here, it would be an assert-like macro that
calls FatalError instead of abort; then the _FATAL cases turn into a
simple xalloc plus a simple assert.

Also:

On Mon, Apr 12, 2010 at 7:54 AM, Tiago Vignatti
<tiago.vignatti at nokia.com> wrote:
> diff --git a/include/misc.h b/include/misc.h
> ...
> +#define _MAXSCREENSALLOCF(o,size,fatal)                                 \
> +    do {                                                                \
> +        if (!o) {                                                       \
> +            o = xalloc((size) * sizeof(*(o)));                 \
> +            if (!o && fatal) FatalError(MAXSCREEN_FAILED_TXT #o);       \
> +        }                                                               \
> +    } while (0)
> ...
> +#define MAXSCREENSCALLOC(o, size, m)          _MAXSCREENSALLOCF(o,size*(m),0)
> +#define MAXSCREENSCALLOC_FATAL(o, size, m)    _MAXSCREENSALLOCF(o,size*(m),1)

The CALLOC variants aren't used anywhere in the patch, which is good,
because I think they're wrong. :-) As far as I can tell, xalloc
doesn't zero the allocated memory, which the standard C library calloc
function does.

The PLUSONE variants are also unused, and seem like API I'd want to
discourage anyone from ever using...

Thanks for working on getting rid of MAXSCREENS, Tiago!

Jamey


More information about the xorg-devel mailing list