[Spice-devel] [PATCH spice-gtk 2/2] gstreamer: Avoid dangling pointers in free_pipeline

Kasireddy, Vivek vivek.kasireddy at intel.com
Fri Nov 3 04:58:34 UTC 2023


Hi Frediano,

> 
> Although currently after free_pipeline we free the entire structure
> the name and the function suggests that we only free the pipeline.
> Also this is fixing a future possible problem with the series
> from Vivek Kasireddy reusing the SpiceGstDecoder for another
> pipeline if H/W encoder pipeline creation fails.
> 
> Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
> ---
>  src/channel-display-gst.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 3bd948d2..ffc2edbf 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -361,11 +361,14 @@ static void free_pipeline(SpiceGstDecoder
> *decoder)
> 
>      gst_element_set_state(decoder->pipeline, GST_STATE_NULL);
>      gst_object_unref(decoder->appsrc);
> +    decoder->appsrc = NULL;
>      if (decoder->appsink) {
Would it make sense to get rid of the above if as well to be consistent?
I don't see why it is needed given that gst_object_unref can accept NULL
as input. Regardless, this patch is
Acked-by: Vivek Kasireddy <vivek.kasireddy at intel.com>

Thanks,
Vivek

>          gst_object_unref(decoder->appsink);
> +        decoder->appsink = NULL;
>      }
> -    gst_object_unref(decoder->pipeline);
>      gst_object_unref(decoder->clock);
> +    decoder->clock = NULL;
> +    gst_object_unref(decoder->pipeline);
>      decoder->pipeline = NULL;
>  }
> 
> --
> 2.41.0



More information about the Spice-devel mailing list