[Mesa-dev] [PATCH] st/va: restore old buffer format on error

[Eric Engestrom]

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

> 
> 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 <christian.koenig at amd.com
> > > <mailto:christian.koenig at amd.com>> wrote:
> > > 
> > >     Am 31.05.2016 um 03:24 schrieb Eric Engestrom:
> > > 
> > >         CoverityID: 1337953
> > > 
> > >         Signed-off-by: Eric Engestrom <eric at engestrom.ch>
> > >         ---
> > > 
> > >         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
> > >     mesa-dev at lists.freedesktop.org
> > >     <mailto:mesa-dev at lists.freedesktop.org>
> > >     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > 
> > > 
> > 
> 

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev