From bef423bee62f7b74858f1b7f74be21405ca75eef Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 26 Jun 2013 15:22:13 -0700 Subject: [PATCH] dri: Choose a decent global driNConfigOptions. Previously, we were asserting that each driver specified an NConfigOptions exactly equal to the number of options they supplied, leading to frequent bugs when people would forget to adjust the value when adjusting driver options. Instead, just overallocate the table by a bit and leave sanity checking to the assert in findOption(). Reviewed-by: Kenneth Graunke --- .../state_trackers/dri/common/dri_screen.c | 5 +-- src/mesa/drivers/dri/common/dri_util.c | 4 +- src/mesa/drivers/dri/common/xmlconfig.c | 44 ++++--------------- src/mesa/drivers/dri/common/xmlconfig.h | 4 +- src/mesa/drivers/dri/i915/intel_screen.c | 5 +-- src/mesa/drivers/dri/i965/intel_screen.c | 5 +-- src/mesa/drivers/dri/radeon/radeon_screen.c | 5 +-- 7 files changed, 14 insertions(+), 58 deletions(-) diff --git a/src/gallium/state_trackers/dri/common/dri_screen.c b/src/gallium/state_trackers/dri/common/dri_screen.c index 3b42b5aa243..779741e32e3 100644 --- a/src/gallium/state_trackers/dri/common/dri_screen.c +++ b/src/gallium/state_trackers/dri/common/dri_screen.c @@ -74,8 +74,6 @@ PUBLIC const char __driConfigOptions[] = #define false 0 -static const uint __driNConfigOptions = 13; - static const __DRIconfig ** dri_fill_in_modes(struct dri_screen *screen) { @@ -417,8 +415,7 @@ dri_init_screen_helper(struct dri_screen *screen, else screen->target = PIPE_TEXTURE_RECT; - driParseOptionInfo(&screen->optionCacheDefaults, - __driConfigOptions, __driNConfigOptions); + driParseOptionInfo(&screen->optionCacheDefaults, __driConfigOptions); driParseConfigFiles(&screen->optionCache, &screen->optionCacheDefaults, diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c index 9ed9df4b39b..fa520ea904d 100644 --- a/src/mesa/drivers/dri/common/dri_util.c +++ b/src/mesa/drivers/dri/common/dri_util.c @@ -52,8 +52,6 @@ PUBLIC const char __dri2ConfigOptions[] = DRI_CONF_SECTION_END DRI_CONF_END; -static const uint __dri2NConfigOptions = 1; - /*****************************************************************/ /** \name Screen handling functions */ /*****************************************************************/ @@ -112,7 +110,7 @@ dri2CreateNewScreen(int scrn, int fd, return NULL; } - driParseOptionInfo(&psp->optionInfo, __dri2ConfigOptions, __dri2NConfigOptions); + driParseOptionInfo(&psp->optionInfo, __dri2ConfigOptions); driParseConfigFiles(&psp->optionCache, &psp->optionInfo, psp->myNum, "dri2"); return psp; diff --git a/src/mesa/drivers/dri/common/xmlconfig.c b/src/mesa/drivers/dri/common/xmlconfig.c index 5c97c20fc0b..b95e452f1ac 100644 --- a/src/mesa/drivers/dri/common/xmlconfig.c +++ b/src/mesa/drivers/dri/common/xmlconfig.c @@ -132,16 +132,6 @@ static GLuint findOption (const driOptionCache *cache, const char *name) { return hash; } -/** \brief Count the real number of options in an option cache */ -static GLuint countOptions (const driOptionCache *cache) { - GLuint size = 1 << cache->tableSize; - GLuint i, count = 0; - for (i = 0; i < size; ++i) - if (cache->info[i].name) - count++; - return count; -} - /** \brief Like strdup but using malloc and with error checking. */ #define XSTRDUP(dest,source) do { \ GLuint len = strlen (source); \ @@ -685,25 +675,18 @@ static void optInfoEndElem (void *userData, const XML_Char *name) { } } -void driParseOptionInfo (driOptionCache *info, - const char *configOptions, GLuint nConfigOptions) { +void driParseOptionInfo (driOptionCache *info, const char *configOptions) { XML_Parser p; int status; struct OptInfoData userData; struct OptInfoData *data = &userData; - GLuint realNoptions; - - /* determine hash table size and allocate memory: - * 3/2 of the number of options, rounded up, so there remains always - * at least one free entry. This is needed for detecting undefined - * options in configuration files without getting a hash table overflow. - * Round this up to a power of two. */ - GLuint minSize = (nConfigOptions*3 + 1) / 2; - GLuint size, log2size; - for (size = 1, log2size = 0; size < minSize; size <<= 1, ++log2size); - info->tableSize = log2size; - info->info = calloc(size, sizeof (driOptionInfo)); - info->values = calloc(size, sizeof (driOptionValue)); + + /* Make the hash table big enough to fit more than the maximum number of + * config options we've ever seen in a driver. + */ + info->tableSize = 6; + info->info = calloc(1 << info->tableSize, sizeof (driOptionInfo)); + info->values = calloc(1 << info->tableSize, sizeof (driOptionValue)); if (info->info == NULL || info->values == NULL) { fprintf (stderr, "%s: %d: out of memory.\n", __FILE__, __LINE__); abort(); @@ -728,17 +711,6 @@ void driParseOptionInfo (driOptionCache *info, XML_FATAL ("%s.", XML_ErrorString(XML_GetErrorCode(p))); XML_ParserFree (p); - - /* Check if the actual number of options matches nConfigOptions. - * A mismatch is not fatal (a hash table overflow would be) but we - * want the driver developer's attention anyway. */ - realNoptions = countOptions (info); - if (realNoptions != nConfigOptions) { - fprintf (stderr, - "Error: nConfigOptions (%u) does not match the actual number of options in\n" - " __driConfigOptions (%u).\n", - nConfigOptions, realNoptions); - } } /** \brief Parser context for configuration files. */ diff --git a/src/mesa/drivers/dri/common/xmlconfig.h b/src/mesa/drivers/dri/common/xmlconfig.h index c363af764fe..d0ad42c1950 100644 --- a/src/mesa/drivers/dri/common/xmlconfig.h +++ b/src/mesa/drivers/dri/common/xmlconfig.h @@ -76,7 +76,6 @@ typedef struct driOptionCache { GLuint tableSize; /**< \brief Size of the arrays * - * Depending on the hash function this may differ from __driNConfigOptions. * In the current implementation it's not actually a size but log2(size). * The value is the same in the screen and all contexts. */ } driOptionCache; @@ -87,14 +86,13 @@ typedef struct driOptionCache { * * \param info pointer to a driOptionCache that will store the option info * \param configOptions XML document describing available configuration opts - * \param nConfigOptions number of options, used to choose a hash table size * * For the option information to be available to external configuration tools * it must be a public symbol __driConfigOptions. It is also passed as a * parameter to driParseOptionInfo in order to avoid driver-independent code * depending on symbols in driver-specific code. */ void driParseOptionInfo (driOptionCache *info, - const char *configOptions, GLuint nConfigOptions); + const char *configOptions); /** \brief Initialize option cache from info and parse configuration files * * To be called in CreateContext. screenNum and driverName select diff --git a/src/mesa/drivers/dri/i915/intel_screen.c b/src/mesa/drivers/dri/i915/intel_screen.c index 30a867e029e..f8b95f44a9d 100644 --- a/src/mesa/drivers/dri/i915/intel_screen.c +++ b/src/mesa/drivers/dri/i915/intel_screen.c @@ -77,8 +77,6 @@ PUBLIC const char __driConfigOptions[] = DRI_CONF_SECTION_END DRI_CONF_END; -const GLuint __driNConfigOptions = 12; - #include "intel_batchbuffer.h" #include "intel_buffers.h" #include "intel_bufmgr.h" @@ -1124,8 +1122,7 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp) return false; } /* parse information in __driConfigOptions */ - driParseOptionInfo(&intelScreen->optionCache, - __driConfigOptions, __driNConfigOptions); + driParseOptionInfo(&intelScreen->optionCache, __driConfigOptions); intelScreen->driScrnPriv = psp; psp->driverPrivate = (void *) intelScreen; diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c index 4ee86026574..9b3c31a5e71 100644 --- a/src/mesa/drivers/dri/i965/intel_screen.c +++ b/src/mesa/drivers/dri/i965/intel_screen.c @@ -77,8 +77,6 @@ PUBLIC const char __driConfigOptions[] = DRI_CONF_SECTION_END DRI_CONF_END; -const GLuint __driNConfigOptions = 12; - #include "intel_batchbuffer.h" #include "intel_buffers.h" #include "intel_bufmgr.h" @@ -1259,8 +1257,7 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp) return false; } /* parse information in __driConfigOptions */ - driParseOptionInfo(&intelScreen->optionCache, - __driConfigOptions, __driNConfigOptions); + driParseOptionInfo(&intelScreen->optionCache, __driConfigOptions); intelScreen->driScrnPriv = psp; psp->driverPrivate = (void *) intelScreen; diff --git a/src/mesa/drivers/dri/radeon/radeon_screen.c b/src/mesa/drivers/dri/radeon/radeon_screen.c index e7a27cf6a6d..dc44d4a105b 100644 --- a/src/mesa/drivers/dri/radeon/radeon_screen.c +++ b/src/mesa/drivers/dri/radeon/radeon_screen.c @@ -95,7 +95,6 @@ DRI_CONF_BEGIN DRI_CONF_NO_RAST("false") DRI_CONF_SECTION_END DRI_CONF_END; -static const GLuint __driNConfigOptions = 14; #elif defined(RADEON_R200) @@ -123,7 +122,6 @@ DRI_CONF_BEGIN DRI_CONF_NO_RAST("false") DRI_CONF_SECTION_END DRI_CONF_END; -static const GLuint __driNConfigOptions = 15; #endif @@ -492,8 +490,7 @@ radeonCreateScreen2(__DRIscreen *sPriv) radeon_init_debug(); /* parse information in __driConfigOptions */ - driParseOptionInfo (&screen->optionCache, - __driConfigOptions, __driNConfigOptions); + driParseOptionInfo (&screen->optionCache, __driConfigOptions); screen->chip_flags = 0; -- 2.30.2