On Wed, Mar 12, 2014 at 4:45 PM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote:
> In theory it's possible for any of the nouveau_getparam calls to
> fail whist the last one being successful.
>
> Thus at least one of the following (hard requirements) drmVersion,
> chipset and vram/gart memory size will be filled with garbage and
> sent to the userspace drivers.
What was wrong with the old logic again? Except annoying indentation?
ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_CHIPSET_ID, &chipset);
if (ret == 0)
ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_FB_SIZE, &vram);
if (ret == 0)
ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_AGP_SIZE, &gart);
if (ret) {
nouveau_device_del(&dev);
return ret;
}
It will only run each successive getparam if the previous one succeeded...
>
> Signed-off-by: Emil Velikov <emil.l.velikov@xxxxxxxxx>
> ---
> nouveau/nouveau.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/nouveau/nouveau.c b/nouveau/nouveau.c
> index ee7893b..d6013db 100644
> --- a/nouveau/nouveau.c
> +++ b/nouveau/nouveau.c
> @@ -88,27 +88,32 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev)
> nvdev->base.fd = fd;
>
> ver = drmGetVersion(fd);
> - if (ver) dev->drm_version = (ver->version_major << 24) |
> - (ver->version_minor << 8) |
> - ver->version_patchlevel;
> + if (!ver) {
> + ret = -errno;
> + goto error;
> + }
> +
> + dev->drm_version = (ver->version_major << 24) |
> + (ver->version_minor << 8) |
> + ver->version_patchlevel;
> drmFreeVersion(ver);
>
> if ( dev->drm_version != 0x00000010 &&
> (dev->drm_version < 0x01000000 ||
> dev->drm_version >= 0x02000000)) {
> - nouveau_device_del(&dev);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_CHIPSET_ID, &chipset);
> - if (ret == 0)
> + if (ret)
> + goto error;
> ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_FB_SIZE, &vram);
> - if (ret == 0)
> + if (ret)
> + goto error;
> ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_AGP_SIZE, &gart);
> - if (ret) {
> - nouveau_device_del(&dev);
> - return ret;
> - }
> + if (ret)
> + goto error;
>
> ret = nouveau_getparam(dev, NOUVEAU_GETPARAM_HAS_BO_USAGE, &bousage);
> if (ret == 0)
> @@ -139,6 +144,9 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev)
>
> *pdev = &nvdev->base;
> return 0;
> +error:
> + nouveau_device_del(&dev);
> + return -ret;
you mean 'ret' of course.
> }
>
> int
> --
> 1.9.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
