Hi Dan,
On Monday, 28 May 2018 13:36:08 EEST Dan Carpenter wrote:
> On Mon, May 28, 2018 at 11:31:01AM +0300, Laurent Pinchart wrote:
> > And that being said, I just tried
> >
> > if (pipe->num_inputs > 2)
> > brx = &vsp1->bru->entity;
> > else if (pipe->brx && !drm_pipe->force_brx_release)
> > brx = pipe->brx;
> > else if (!vsp1->bru->entity.pipe)
> > brx = &vsp1->bru->entity;
> > else
> > brx = &vsp1->brs->entity;
> >
> > if (!brx)
> > return -EINVAL;
> >
> > and that didn't help either... Dan, would you have some light to shed on
> > this problem ?
>
> This is a problem in Smatch.
>
> We should be able to go backwards and say that "If we know 'brx' is
> non-NULL then let's mark &vsp1->brs->entity, vsp1->brs,
> &vsp1->bru->entity and vsp1->bru all as non-NULL as well". But Smatch
> doesn't go backwards like that. The information is mostly there to do
> it, but my instinct is that it's really hard to implement.
>
> The other potential problem here is that Smatch stores comparisons and
> values separately. In other words smatch_comparison.c has all the
> information about brx == &vsp1->bru->entity and smatch_extra.c has the
> information about if brx is NULL or non-NULL. They don't really share
> information very well.
It would indeed be useful to implement, but I share your concern that this
would be pretty difficult.
However, there's still something that puzzles me. Let's add a bit more
context.
if (pipe->num_inputs > 2)
brx = &vsp1->bru->entity;
else if (pipe->brx && !drm_pipe->force_brx_release)
brx = pipe->brx;
else if (!vsp1->bru->entity.pipe)
brx = &vsp1->bru->entity;
else
brx = &vsp1->brs->entity;
1. if (!brx)
return -EINVAL;
2. if (brx != pipe->brx) {
...
3. pipe->brx = brx;
...
}
4. format.pad = pipe->brx->source_pad
(1) ensures that brx can't be NULL. (2) is thus always true if pipe->brx is
NULL. (3) then assigns a non-NULL value to pipe->brx. Smatch should thus never
complain about (4), even if it can't backtrack.
--
Regards,
Laurent Pinchart
