Hi Yunke,
On Thu, Apr 14, 2022 at 03:44:47PM +0900, Yunke Cao wrote:
> On Thu, Apr 14, 2022 at 3:19 PM Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx> wrote:
> >
> > Hi Yunke,
> >
> > Thanks for the patch.
> >
> > On Tue, Apr 12, 2022 at 03:23:13PM +0900, Yunke Cao wrote:
> > > The original implementation removes reverse links for any input link and
> > > assumes the presense of sink/source.
> > > It fails when the link is a not a data link.
> > > media_entity_remove_links when there's an ancillary link can also fail.
> >
> > The function's return type is void. Are there other adverse effects from
> > this? Looking at the function, it would seem like that the reverse link
> > simply isn't found in this case, and so not removed.
> >
> The function dereferences without any check link->source and link->sink
> ("link->source->entity == entity" etc.), which is in union.
> Ancillary links populate gobj0/gobj1 instead of source/sink.
> Calling this function on ancillary links can cause crashes.
Indeed. The struct definition isn't too pretty either. And this only works
with interface links as they're not included in the links list...
> > >
> > > We only need to remove reverse links for a data link.
> >
> > Ideally this would not be based on the link flags as it's not a very robust
> > way to test whather a backlink needs to be removed.
> >
> I was mainly trying to make sure link->source and link->sink are
> populated by checking the link type.
> Currently, only data links need to run this part of the code to remove
> reverse links so I feel this is
> the easiest way. Let me know if there's any better alternative.
I think we should get this in now. The code seems to be in a dire need for
cleanup still.
--
Regards,
Sakari Ailus
