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 <[email protected]> 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
