[Libva] [PATCH 0/2] add user specified tiling/stride support, clean up assert

[Zhao, Halley]

Thanks for your comments.

The 2nd patch is so big that it is blocked by mail man, so I sent to some stake holders for review.
Your VA_INTEL_DEBUG=asserts sounds good, we can consider it.


-----Original Message-----
From: Gwenole Beauchesne [mailto:gb.devel at gmail.com] 
Sent: Wednesday, March 19, 2014 12:52 PM
To: Zhao, Halley
Cc: libva at lists.freedesktop.org
Subject: Re: [Libva] [PATCH 0/2] add user specified tiling/stride support, clean up assert

Hi,

2014-03-14 10:19 GMT+01:00 Zhao, Halley <halley.zhao at intel.com>:

> when I implements feature for user specified tiling/stride support, 
> haihao mentioned that I'd better 'return fail' instead of assert().
> so, I tried to clean up the assert in i965_drv_video.c as well.

The assert_ret() patch did not appear on the list but was committed as 12c8122.

What Haihao mentioned was right, but what I had precisely indicated beforehand was to not have the VA driver fail at all in the default case too. Besides, the proposed patch turned down the indication of what went wrong. i.e. assert(0) is totally meaningless, unless you get the original source code the binary was built with, and look for the specified file/line number information.

The proper approach would have been to log the error in any case, possibly controlled by an environment variable (e.g.
VA_INTEL_DEBUG=asserts), but definitely not an assert(0) again. That way, you can still get informative enough bug reports. And, most importantly, you also get a chance to test/exercice the error handling code of upper layers in the general case too.

Thanks,
Gwenole.