On Sunday, November 27, 2016 12:17:52 PM PST Emil Velikov wrote:
> On 27 November 2016 at 02:31, Kenneth Graunke <kenn...@whitecape.org> wrote:
> > On Thursday, November 24, 2016 8:30:39 PM PST Emil Velikov wrote:
> >> From: Emil Velikov <emil.veli...@collabora.com>
[snip]
> >> @@ -186,7 +208,14 @@ anv_physical_device_init(struct anv_physical_device
> >> *device,
> >> if (result != VK_SUCCESS)
> >> goto fail;
> >>
> >> - anv_device_get_cache_uuid(device->uuid);
> >> + if (anv_device_get_cache_uuid(device->uuid)) {
> >> + anv_finish_wsi(device);
> >> + ralloc_free(device->compiler);
> >> + result = vk_errorf(VK_ERROR_INITIALIZATION_FAILED,
> >> + "cannot generate UUID");
> >> + goto fail;
> >> + }
> >> +
> >
> > I don't see why this error case is special - all the others just have
> > "goto fail". This stuff may need to get cleaned up, but shouldn't it
> > happen for the other error paths too?
> >
> We need to set the error, since result is equal to VK_SUCCESS as we
> can (barely) see in the diff above. If you want we can make
> anv_device_get_cache_uuid() return VkResult and honour that ?
>
> What do you mean with "all the others just have goto fail" ? Most
> error paths have appropriate teardown with the incomplete ones being
> addressed later in the series.
> Are you thinking of more fine-grained goto labels or something else ?
I was looking at the previous two. device->compiler == NULL apparently
doesn't need to perform any teardown. But anv_init_wsi failing ought to
be freeing device->compiler, and it isn't. That's a bug I suppose.
Rather than adding anv_finish_wsi/ralloc_free calls here, why not move
the cache UUID initialization earlier? (Say, after the MMAP_VERSION
check).
Then we can just do
if (!anv_device_get_cache_uuid(device->uuid, device->chipset_id)) {
result = vkerrorf(VK_ERROR_INITIALIZATION_FAILED,
"failed to get cache UUID");
goto fail;
}
Or make anv_device_get_cache_uuid return VkSuccess/vkerrorf. I like
that idea too...it's clearer than a boolean or an integer.
I'm fine with whatever you come up with.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
