On Wed, Mar 12, 2014 at 9:04 PM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote:
> On 13/03/14 00:45, Ilia Mirkin wrote:
>>
>> 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...
>>
> Good point, the lovely indentation got me. So it seems only !ver handling is
> needed.
Not really. drm->drm_version will be 0 if ver fails.
>
>
>>
>>>
>>> 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.
>>
> Yikes, thanks.
>
> -Emil
>
>
>>> }
>>>
>>> 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
