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

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