Re: [PATCH libdrm v3 3/5] xf86drm: Add platform and host1x bus support

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2398 bytes --]

Adding back dri-devel@lists.freedesktop.org

On Wed, Jan 18, 2017 at 04:00:20PM +0100, walter harms wrote:
> Am 18.01.2017 10:02, schrieb Thierry Reding:
[...]
> > diff --git a/xf86drm.c b/xf86drm.c
[...]
> > @@ -3187,11 +3199,55 @@ static int drmParsePciDeviceInfo(int maj, int min,
> >  #endif
> >  }
> >  
> > +static void drmFreePlatformDevice(drmDevicePtr device)
> > +{
> > +    if (device->deviceinfo.platform) {
> 
> to save 2 indent level and improve readability :
> 
> if (! device->deviceinfo.platform ||
>     ! device->deviceinfo.platform->compatible)
> 	return;
> 
> > +        if (device->deviceinfo.platform->compatible) {
> > +            char **compatible = device->deviceinfo.platform->compatible;
> > +
> > +            while (*compatible) {
> > +                free(*compatible);
> > +                compatible++;
> > +            }
> > +
> > +            free(device->deviceinfo.platform->compatible);
> > +        }
> > +    }

I chose the above style because I thought readability wasn't impacted
and it has the advantage of making the code more easily extensible
should we ever need to add code to it.

> > +static int drmProcessHost1xDevice(drmDevicePtr *device,
> > +                                  const char *node, int node_type,
> > +                                  int maj, int min, bool fetch_deviceinfo,
> > +                                  uint32_t flags)
> > +{
> > +    drmDevicePtr dev;
> > +    char *ptr;
> > +    int ret;
> > +
> > +    dev = drmDeviceAlloc(node_type, node, sizeof(drmHost1xBusInfo),
> > +                         sizeof(drmHost1xDeviceInfo), &ptr);
> > +    if (!dev)
> > +        return -ENOMEM;
> > +
> > +    dev->bustype = DRM_BUS_HOST1X;
> > +
> > +    dev->businfo.host1x = (drmHost1xBusInfoPtr)ptr;
> > +
> > +    ret = drmParseHost1xBusInfo(maj, min, dev->businfo.host1x);
> > +    if (ret < 0)
> > +        goto free_device;
> > +
> > +    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?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel