Jordan Justen <jljus...@gmail.com> writes: >> We find the driver foo's name by using the dladdr function >> which gives the path of the dynamic library's name that >> was being loaded.
That sounds like all kinds of win for existing X servers. Thanks for
doing it up in style, so that a megadrivers build can actually work for
all chips in the megadriver.
(as a style issue, I probably would have computed the driver name from
the dli_fname value directly and then copied it out into new storage, or
directly into get_extensions_name, but that's just me, so you should
feel free to ignore it :-)
>> + /* Make sure the patch ends with _dri.so */
>> + name_len = strlen(driver_name);
>> + if (strcmp(driver_name + (name_len - 7), "_dri.so") != 0) {
Need to make sure name_len is >= 7 here. Should probably just have a
test that checks namelen and bails if it's < 7 as there are other places
using this magic value.
(Oh, I'd probably stick "_dri.so" in a #define and then #define the
length of it too, instead of using '7' in several places. Again, style,
not substance, so you can ignore that as you please.)
>> + i = asprintf(&get_extensions_name, "%s_%s",
>> + __DRI_DRIVER_GET_EXTENSIONS, driver_name);
>> + free(driver_path);
>> + if (i == -1 || !get_extensions_name)
>> + return;
Is the null pointer check here useful or necessary? asprintf doesn't
define the value when allocation fails, preferring to return -1
instead. Are there systems which return valid 'i' and null pointer?
>> + /* Copy the extensions into the __driDriverExtensions array
>> + * we declared.
>> + */
>> + for (i = 0; i < ARRAY_SIZE(__driDriverExtensions); i++) {
>> + __driDriverExtensions[i] = extensions[i];
>> + if (extensions[i] == NULL)
>> + break;
>> + }
>> +
>> + /* If the driver had more extensions that we reserved, then
>> + * bail out. This will cause the driver to fail to load using
>> + * the older loader mechanism.
>> + */
>> + if (extensions[i] != NULL) {
This check is incorrect -- you will dereference off the end of the array
when you fill it. Instead, you should just check for
if (i == ARRAY_SIZE(__driDriverExtensions))
as that will let you know the array was filled
--
keith.pack...@intel.com
pgpBROPNRy1Ba.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
