Re: [PATCH v3 2/9] add vnc h264 encoder

[Dietmar Maurer]

> > > > +void vnc_h264_clear(VncState *vs)
> > > > +{
> > > > +    if (!vs->h264) {
> > > > +        return;
> > > > +    }
> > >
> > > unnecessary
> >
> > This is required. For example if you disable h264, vs->h264 is
> > always NULL, and we unconditionally call vnc_h264_clear().
> >
> > Why do you think this is unnecessary?
> 
> and there are already checks for NULL, no need to do it twice, do it
> where it is actually necessary.

There is no check in destroy_encoder_context(), so this will generate a core 
dump.

So what do you mean by "where it is actually necessary"?

The final code looks like:

void vnc_h264_clear(VncState *vs)
{
    // Assume we remove this check ...
    // if (!vs->h264) {
    //    return;
    //}

    // will trigger a core dump
    notifier_remove(&vs->h264->shutdown_notifier);

    // will trigger a core dump
    destroy_encoder_context(vs->h264);
    // will trigger a core dump
    g_free(vs->h264->encoder_name);

    g_clear_pointer(&vs->h264, g_free);
}

Where do you want the check for NULL exactly? At the call site?

- Dietmar