[PATCH v2 1/5] xfree86: Make driver matching consistent
Aaron Plattner
aplattner at nvidia.com
Wed Feb 19 13:27:12 PST 2014
On 02/18/2014 02:19 PM, Thierry Reding wrote:
> From: Thierry Reding <treding at nvidia.com>
>
> Most of the driver enumeration functions take an array and a maximum
> number of entries that they are allowed to fill in. Upon success, they
> return the number of entries filled in. This allows them to be easily
> used to consecutively.
>
> One exception is the xf86MatchDriverFromFiles() function, which doesn't
> return a value, so callers have to manually search the array for the
> first empty entry.
>
> This commit modifies the xf86MatchDriverFromFiles() to behave the same
> way as others, which makes it easier to deal with.
Wow, this nmatches accounting is all screwy. It looks like
xf86AutoConfig expects the deviceList array to have a NULL sentinel, and
xf86MatchDriverFromFiles is careful to check for (i < (nmatches - 1))
rather than just (i < nmatches). However, xf86VideoPtrToDriverList will
happily write to the very end of the array and xf86MatchDriverFromFiles
totally ignores its nmatches argument.
I don't think it needs to be fixed as part of this change, but it would
make a lot more sense for all of the checks to be for (i < nmatches),
and for listPossibleVideoDrivers to just return the actual number of
matches found.
Since those are preexisting problems,
Reviewed-By: Aaron Plattner <aplattner at nvidia.com>
Tested-By: Aaron Plattner <aplattner at nvidia.com>
-- Aaron
> Signed-off-by: Thierry Reding <treding at nvidia.com>
> ---
> Changes in v2:
> - new patch
>
> hw/xfree86/common/xf86AutoConfig.c | 2 +-
> hw/xfree86/common/xf86pciBus.c | 24 ++++++++++--------------
> hw/xfree86/common/xf86pciBus.h | 5 +++--
> hw/xfree86/common/xf86platformBus.c | 7 ++-----
> 4 files changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
> index 4eb86de22f8d..2b53b908a591 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -265,7 +265,7 @@ listPossibleVideoDrivers(char *matches[], int nmatches)
> #endif
> #ifdef XSERVER_LIBPCIACCESS
> if (i < (nmatches - 1))
> - i = xf86PciMatchDriver(matches, nmatches);
> + i += xf86PciMatchDriver(&matches[i], nmatches - i);
> #endif
>
> #if defined(__linux__)
> diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
> index 0f76a03ee7e7..c06b04033238 100644
> --- a/hw/xfree86/common/xf86pciBus.c
> +++ b/hw/xfree86/common/xf86pciBus.c
> @@ -1320,8 +1320,9 @@ xchomp(char *line)
> * don't export their PCI ID's properly. If distros don't end up using this
> * feature it can and should be removed because the symbol-based resolution
> * scheme should be the primary one */
> -void
> -xf86MatchDriverFromFiles(char **matches, uint16_t match_vendor, uint16_t match_chip)
> +int
> +xf86MatchDriverFromFiles(uint16_t match_vendor, uint16_t match_chip,
> + char *matches[], int nmatches)
> {
> DIR *idsdir;
> FILE *fp;
> @@ -1331,11 +1332,11 @@ xf86MatchDriverFromFiles(char **matches, uint16_t match_vendor, uint16_t match_c
> ssize_t read;
> char path_name[256], vendor_str[5], chip_str[5];
> uint16_t vendor, chip;
> - int i, j;
> + int i = 0, j;
>
> idsdir = opendir(PCI_TXT_IDS_PATH);
> if (!idsdir)
> - return;
> + return 0;
>
> xf86Msg(X_INFO,
> "Scanning %s directory for additional PCI ID's supported by the drivers\n",
> @@ -1386,10 +1387,6 @@ xf86MatchDriverFromFiles(char **matches, uint16_t match_vendor, uint16_t match_c
> }
> }
> if (vendor == match_vendor && chip == match_chip) {
> - i = 0;
> - while (matches[i]) {
> - i++;
> - }
> matches[i] =
> (char *) malloc(sizeof(char) *
> strlen(direntry->d_name) - 3);
> @@ -1412,6 +1409,7 @@ xf86MatchDriverFromFiles(char **matches, uint16_t match_vendor, uint16_t match_c
> }
> xf86Msg(X_INFO, "Matched %s from file name %s\n",
> matches[i], direntry->d_name);
> + i++;
> }
> }
> else {
> @@ -1425,6 +1423,7 @@ xf86MatchDriverFromFiles(char **matches, uint16_t match_vendor, uint16_t match_c
> end:
> free(line);
> closedir(idsdir);
> + return i;
> }
> #endif /* __linux__ */
>
> @@ -1435,7 +1434,7 @@ xf86MatchDriverFromFiles(char **matches, uint16_t match_vendor, uint16_t match_c
> int
> xf86PciMatchDriver(char *matches[], int nmatches)
> {
> - int i;
> + int i = 0;
> struct pci_device *info = NULL;
> struct pci_device_iterator *iter;
>
> @@ -1450,13 +1449,10 @@ xf86PciMatchDriver(char *matches[], int nmatches)
> pci_iterator_destroy(iter);
> #ifdef __linux__
> if (info)
> - xf86MatchDriverFromFiles(matches, info->vendor_id, info->device_id);
> + i += xf86MatchDriverFromFiles(info->vendor_id, info->device_id,
> + matches, nmatches);
> #endif
>
> - for (i = 0; (i < nmatches) && (matches[i]); i++) {
> - /* find end of matches list */
> - }
> -
> if ((info != NULL) && (i < nmatches)) {
> i += xf86VideoPtrToDriverList(info, &(matches[i]), nmatches - i);
> }
> diff --git a/hw/xfree86/common/xf86pciBus.h b/hw/xfree86/common/xf86pciBus.h
> index b497a7f2df9d..45b5a0feefcc 100644
> --- a/hw/xfree86/common/xf86pciBus.h
> +++ b/hw/xfree86/common/xf86pciBus.h
> @@ -47,8 +47,9 @@ void xf86PciConfigureNewDev(void *busData, struct pci_device *pVideo,
> ((x)->func == (y)->func) && \
> ((x)->dev == (y)->dev))
>
> -void
> -xf86MatchDriverFromFiles(char **matches, uint16_t match_vendor, uint16_t match_chip);
> +int
> +xf86MatchDriverFromFiles(uint16_t match_vendor, uint16_t match_chip,
> + char *matches[], int nmatches);
> int
> xf86VideoPtrToDriverList(struct pci_device *dev,
> char *returnList[], int returnListMax);
> diff --git a/hw/xfree86/common/xf86platformBus.c b/hw/xfree86/common/xf86platformBus.c
> index 5875a91363cc..3da6ae19ba90 100644
> --- a/hw/xfree86/common/xf86platformBus.c
> +++ b/hw/xfree86/common/xf86platformBus.c
> @@ -198,13 +198,10 @@ xf86PlatformMatchDriver(char *matches[], int nmatches)
> info = xf86_platform_devices[i].pdev;
> #ifdef __linux__
> if (info)
> - xf86MatchDriverFromFiles(matches, info->vendor_id, info->device_id);
> + j += xf86MatchDriverFromFiles(info->vendor_id, info->device_id,
> + &matches[j], nmatches - j);
> #endif
>
> - for (j = 0; (j < nmatches) && (matches[j]); j++) {
> - /* find end of matches list */
> - }
> -
> if ((info != NULL) && (j < nmatches)) {
> j += xf86VideoPtrToDriverList(info, &(matches[j]), nmatches - j);
> }
>
--
Aaron
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
More information about the xorg-devel
mailing list