[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg-devel
Subject:    Re: [PATCH libdrm v3 3/5] xf86drm: Add platform and host1x bus support
From:       Emil Velikov <emil.l.velikov () gmail ! com>
Date:       2017-01-22 15:00:32
Message-ID: CACvgo53ovcyD-u8m5PHRfi+z+NNoSdJdmhEz6+Azb4wE6h_L+w () mail ! gmail ! com
[Download RAW message or body]

On 21 January 2017 at 11:07, walter harms <wharms@bfs.de> wrote:

>>>> +    if (fetch_deviceinfo) {
>>>> +        ptr += sizeof(drmHost1xBusInfo);
>>>> +        dev->deviceinfo.host1x = (drmHost1xDeviceInfoPtr)ptr;
>>>> +
>>>> +        ret = drmParseHost1xDeviceInfo(maj, min, dev->deviceinfo.host1x);
>>>> +        if (ret < 0)
>>>> +            goto free_device;
>>>> +    }
>>>> +
>>>> +    *device = dev;
>>>> +
>>>
>>> do you assume that fetch_deviceinfo may change dev ?
>>
>> No, why?
>
> You do *device = dev; after the if block that leaves the impression (for me)
> that dev may change somewhere. therefor i would suggest moving this upwards.
>
Suspecting that you're missing the "Thou Shalt Not overwrite any
output variables on error."

Thus having *device = dev just before "return success" is what you want to do.

-Emil
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[prev in list] [next in list] [prev in thread] [next in thread]