[PATCH] Automatically load video drivers based on PCI ID's

[David Nusinow]

Hi Aaron,

First off, thank you again for reviewing this, I really appreciate it. I've
attached a diff to my patch stack that covers these changes.

On Thu, Feb 21, 2008 at 06:17:39PM -0800, Aaron Plattner wrote:
> Sorry to take so long to look at this again.  This looks better, I just
> have a few remaining suggestions.

No problem with the time, I've actually been too busy to do anything for
the past few weeks anyhow, so this is perfect timing.

> I'd check that strlen(direntry->d_name) > 3, or else bad stuff could happen
> when you do strcmp(&direntry->d_name[len-3], ".so").
> 
> 
> strncpy followed by strlen is not safe if the source string (in this case
> direntry->d_name) is too long because it won't write a terminating '\0'.

Both done, these are great catches. The latter I didn't know about before.

> You've got an extraneous "drec = NULL" right before "drec =
> (DriverPtr)...".

Removed, thanks.

> Instead of
> 
>         retval = (char*)xalloc(strlen(drec->driverName));
>         strcpy(retval, drec->driverName);
> 
> you could simply do
> 
>         retval = xstrdup(direc->driverName);
> 
> 
> For constructing the driver path, XNFprintf looks pretty convenient, if it
> works:
> 
>         driverpath = XNFprintf("%s/drivers/", xf86ModulePath);
> 
> Otherwise, you should probably use xnfalloc or better yet, fail gracefully
> if xalloc fails.

These are both great. I'm not as aware of the utility functions scattered
about the server as I should be. And yes, XNFprintf worked perfectly, so I
used it in another place to get rid of some boilerplate code.

> I don't think
> 
>         char *matches[matchmax]
> 
> works on all compilers, though I could be wrong.

Hrm, I don't know of any other way to define an array of strings. K & R
does this, so any C compiler of worth should support it. If you have any
alternate suggestions though, I'd love to hear them.

 - David Nusinow
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-Tighten-up-pci-id-based-driver-loading-code.patch
Type: text/x-diff
Size: 3674 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20080223/3707ed9b/attachment.patch>