Em Tue, 11 Aug 2015 12:49:40 +0200
Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> On 08/07/15 16:20, Mauro Carvalho Chehab wrote:
> > Using memory realloc to increase the size of an array
> > is complex and makes harder to remove links. Also, by
> > embedding the link inside an array at the entity makes harder
> > to change the code to add interfaces, as interfaces will
> > also need to use links.
> >
> > So, convert the links from arrays to lists.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
> >
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index 9fb3f8958265..a95ca981aabb 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
>
> <snip>
>
> > @@ -452,27 +445,21 @@ EXPORT_SYMBOL_GPL(media_entity_put);
> >
> > static struct media_link *media_entity_add_link(struct media_entity *entity)
> > {
> > - if (entity->num_links >= entity->max_links) {
> > - struct media_link *links = entity->links;
> > - unsigned int max_links = entity->max_links + 2;
> > - unsigned int i;
> > + struct media_link *link;
> >
> > - links = krealloc(links, max_links * sizeof(*links), GFP_KERNEL);
> > - if (links == NULL)
> > - return NULL;
> > + link = kzalloc(sizeof(*link), GFP_KERNEL);
> > + if (link == NULL)
> > + return NULL;
> >
> > - for (i = 0; i < entity->num_links; i++)
> > - links[i].reverse->reverse = &links[i];
> > -
> > - entity->max_links = max_links;
> > - entity->links = links;
> > - }
> > + link->reverse->reverse = link;
>
> Huh? link points to a zeroed struct, so link->reverse will be NULL.
> This can't work.
>
> Are you sure this line should be here? The original code doesn't set it
> either for the new link, it just updates the reverse links for the
> realloced links.
You're right. I think I can just remove that line.
I had to confess that I didn't understand well that the reverse links
stuff. I mean, IMHO, the entire concept of storing a second copy of
the link, calling it as "reverse" on some places and as "backlink"
on others is messy, and I guess we should get rid of duplicating the
number of links for no good reason.
It is easy to just add the same link to a list at the other entity,
and keep the link just once in the memory.
>
> > + INIT_LIST_HEAD(&link->list);
> > + list_add(&entity->links, &link->list);
> >
> > /* Initialize graph object embedded at the new link */
> > graph_obj_init(entity->parent, MEDIA_GRAPH_LINK,
> > - &entity->links[entity->num_links].graph_obj);
> > + &link->graph_obj);
> >
> > - return &entity->links[entity->num_links++];
> > + return link;
> > }
> >
> > int
>
> Regards,
>
> Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
