[PATCH 1/3] nouveau: cleanup error handling during nouveau_device_wrap

[Ilia Mirkin]

On Wed, Mar 12, 2014 at 4:45 PM, Emil Velikov <emil.l.velikov at gmail.com> 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 at gmail.com>
> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel