Re: [PATCH 04/26] drm/rcar-du: Use for_each_*_in_state

Laurent Pinchart
2016-06-03T09:40:02Z

Hi Daniel,

On Friday 03 Jun 2016 08:55:36 Daniel Vetter wrote:
> On Fri, Jun 03, 2016 at 01:54:44AM +0300, Laurent Pinchart wrote:
> > On Monday 30 May 2016 16:54:10 Daniel Vetter wrote:
> >> On Mon, May 30, 2016 at 11:58:27AM +0200, Maarten Lankhorst wrote:
> >>> Op 30-05-16 om 11:18 schreef Laurent Pinchart:
> >>>> Hi Daniel,
> >>>>
> >>>> Thank you for the patch.
> >>>>
> >>>> This looks good to me as the resulting code is mostly similar.
> >>>> However, the for_each_*_in_state macros end with an for_each_if()
> >>>> that tests if the object's state is NULL, which isn't present in this
> >>>> code. I'm wondering whether that was an oversight on my side possibly
> >>>> leading to a crash when dereferencing a NULL state, or an unneeded
> >>>> check in the macros. Can atomic_state->*_states[i] be NULL if
> >>>> atomic_state->*[i] is not NULL ?
> >>>
> >>> Not in any normal case.
> >>
> >> Yeah, the drm_atomic_get_*_state functions only ever fill in both of
> >> neither. If this gets out of sync it's a bug ;-)
> >
> > Should the check be removed then ? Or replaced by a WARN_ON() ?
>
> In all the places I converted here I nuked those checks, since they moved
> into the loop now. Not sure what checks you're talking about.

I'm talking about the for_each_if() check inside the for_each_*_in_state
macros.

--
Regards,

Laurent Pinchart