[PATCH weston 1/7] compositor-drm: remove superfluos get_disable_state call

Ucan, Emre (ADITG/ESB) eucan at de.adit-jv.com
Tue Mar 20 15:03:53 UTC 2018


Hi Daniel,

> -----Original Message-----
> From: Daniel Stone [mailto:daniel at fooishbar.org]
> Sent: Dienstag, 20. März 2018 15:48
> To: Ucan, Emre (ADITG/ESB)
> Cc: wayland-devel at lists.freedesktop.org
> Subject: Re: [PATCH weston 1/7] compositor-drm: remove superfluos
> get_disable_state call
> 
> Hi Emre,
> 
> On 20 March 2018 at 14:28, Emre Ucan <eucan at de.adit-jv.com> wrote:
> > drm_output_get_disable_state function returns
> > a duplicated output_state object.
> >
> > Here we are creating the object, but we are
> > never using it. Therefore, it is safe to remove.
> >
> > (Found by clang source code analyzer)
> 
> It's subtle, but we do need this here.
> 
> > @@ -3860,7 +3860,6 @@ drm_set_dpms(struct weston_output
> *output_base, enum dpms_enum level)
> 
> It's out of diff context, the if branch we're in guarantees that we
> have pending_state, i.e. after repaint_begin() but before
> repaint_flush().
> 
> >                 state = drm_pending_state_get_output(pending_state, output);
> >                 if (state)
> >                         drm_output_state_free(state);
> 
> Here we search pending_state for any existing output state for this
> output. If there are any, we free them and remove them from the
> pending state. So after this line, we are guaranteed not to be
> touching the output in this pending state.
> 
> > -               state = drm_output_get_disable_state(pending_state, output);
> 
> This line adds a disable state to the pending_state. We do not do
> anything with it: we just keep it there in the pending_state, to be
> applied when repaint_flush() is called. If we remove this, then we
> won't disable the output.

Oh, I missed that. 
> 
> Replacing 'state = ' with '(void) ' as the prefix of this line might
> make it clearer; does that also quiet Coverity?

Yes, adding '(void)' as the prefix of this line fixes the issue. I can send a second version of my patches after initial review, or you can fix this anyway, and we can ignore my first patch.
 
> 
> Cheers,
> Daniel

Best Regards,
Emre


More information about the wayland-devel mailing list