<div dir="ltr">no problem, it was actually good, I think that even if current code is correct, I'll still submit the new diff for review because it looks better. <div><div class="gmail_extra"> <div class="gmail_quote">On 2 June 2016 at 10:34, Eric Engestrom <<a href="mailto:eric.engestrom@imgtec.com" target="_blank">eric.engestrom@imgtec.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, May 31, 2016 at 02:54:06PM +0200, Christian König wrote: > On the other hand the original code was correct in the first place. > > And as Julien noted Coverity tries to restore the template buffer format > from a local variable which doesn't exists any more where you wanted to add > the line. > > So Coverity creates code which won't compile and needs to be fixed instead. I guess the lesson for me is to not try and fix stuff in code I don't understand at all :] Sorry for wasting your time with this. Cheers, Eric <div><div class="h5"> > > Christian. > > Am 31.05.2016 um 14:51 schrieb Christian König: > > Yeah, that solution looks more correct to me. > > > > Christian. > > > > Am 31.05.2016 um 14:44 schrieb Julien Isorce: > > > Hi, > > > Thx for looking at it but are you sure your diff compiles ? > > > > > > Can you try this instead: > > > > > > --- a/src/gallium/state_trackers/va/image.c > > > +++ b/src/gallium/state_trackers/va/image.c > > > @@ -471,19 +471,19 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID > > > surface, VAImageID image, > > > > > > if (format != surf->buffer->buffer_format) { > > > struct pipe_video_buffer *tmp_buf; > > > - enum pipe_format old_surf_format = surf->templat.buffer_format; > > > + struct pipe_video_buffer templat = surf->templat; > > > > > > - surf->templat.buffer_format = format; > > > - tmp_buf = drv->pipe->create_video_buffer(drv->pipe, > > > &surf->templat); > > > + templat.buffer_format = format; > > > + tmp_buf = drv->pipe->create_video_buffer(drv->pipe, &templat); > > > > > > if (!tmp_buf) { > > > - surf->templat.buffer_format = old_surf_format; > > > pipe_mutex_unlock(drv->mutex); > > > return VA_STATUS_ERROR_ALLOCATION_FAILED; > > > } > > > > > > surf->buffer->destroy(surf->buffer); > > > surf->buffer = tmp_buf; > > > + surf->templat.buffer_format = format; > > > } > > > > > > Cheers > > > Julien > > > > > > > > > On 31 May 2016 at 08:43, Christian König <<a href="mailto:christian.koenig@amd.com">christian.koenig@amd.com</a> </div></div><div><div class="h5">> > > <mailto:<a href="mailto:christian.koenig@amd.com">christian.koenig@amd.com</a>>> wrote: > > > > > > Am 31.05.2016 um 03:24 schrieb Eric Engestrom: > > > > > > CoverityID: 1337953 > > > > > > Signed-off-by: Eric Engestrom <<a href="mailto:eric@engestrom.ch">eric@engestrom.ch</a>> > > > --- > > > > > > Note that I do not know this code at all; I'm blindly > > > following Coverity's advice on this one :] > > > > > > > > > Well and that is completely nonsense. The buffer was already > > > reallocated when this error happens and so resetting the template > > > to the original value is incorrect and actually rather dangerous. > > > > > > Why does Coverity things that we should add this? And how can we > > > fix this? > > > > > > Regards, > > > Christian. > > > > > > > > > --- > > > src/gallium/state_trackers/va/image.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/src/gallium/state_trackers/va/image.c > > > b/src/gallium/state_trackers/va/image.c > > > index 92d014c..8cfe17a 100644 > > > --- a/src/gallium/state_trackers/va/image.c > > > +++ b/src/gallium/state_trackers/va/image.c > > > @@ -490,6 +490,7 @@ vlVaPutImage(VADriverContextP ctx, > > > VASurfaceID surface, VAImageID image, > > > views = > > > surf->buffer->get_sampler_view_planes(surf->buffer); > > > if (!views) { > > > + surf->templat.buffer_format = old_surf_format; > > > pipe_mutex_unlock(drv->mutex); > > > return VA_STATUS_ERROR_OPERATION_FAILED; > > > } > > > > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> </div></div>> > > <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a>> > > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> <div class="HOEnZb"><div class="h5">> > > > > > > > > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div></div>