All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Linux Media Mailing List <linux-media@vger.kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Prabhakar Lad <prabhakar.csengg@gmail.com>,
	Scott Jiang <scott.jiang.linux@gmail.com>,
	Boris BREZILLON <boris.brezillon@free-electrons.com>
Subject: Re: [PATCH RFC v2 08/16] media: convert links from array to list
Date: Tue, 11 Aug 2015 12:49:40 +0200	[thread overview]
Message-ID: <55C9D344.8090905@xs4all.nl> (raw)
In-Reply-To: <65340c7d01bdfcadbb82f92d63a3571871d07930.1438954897.git.mchehab@osg.samsung.com>

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@osg.samsung.com>
> 
> 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.

> +	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

  reply	other threads:[~2015-08-11 10:52 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-07 14:19 [PATCH RFC v2 00/16] Changes on MC core due to MC workshop discussion Mauro Carvalho Chehab
2015-08-07 14:19 ` [PATCH RFC v2 01/16] media: Add some fields to store graph objects Mauro Carvalho Chehab
2015-08-07 23:14   ` Sakari Ailus
2015-08-07 23:55     ` Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 02/16] media: Add a common embeed struct for all media " Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 03/16] media: add functions to inialize media_graph_obj Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 04/16] media: ensure that entities will have an object ID Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 05/16] media: initialize PAD objects Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 06/16] media: initialize the graph object inside the media links Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 07/16] media: get rid of unused "extra_links" param on media_entity_init() Mauro Carvalho Chehab
2015-08-07 14:20   ` Mauro Carvalho Chehab
2015-08-07 14:20   ` Mauro Carvalho Chehab
2015-08-07 14:20   ` Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 08/16] media: convert links from array to list Mauro Carvalho Chehab
2015-08-11 10:49   ` Hans Verkuil [this message]
2015-08-11 11:12     ` Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 11/16] FIXUP: source link removal on failure Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 12/16] media: move __media_entity_remove_link to avoid prototype Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 13/16] media: make the internal function to create links more generic Mauro Carvalho Chehab
2015-08-11 10:57   ` Hans Verkuil
2015-08-11 11:16     ` Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 14/16] media: add a generic function to remove a link Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 15/16] media: rename media_entity_remove_foo functions Mauro Carvalho Chehab
2015-08-07 14:20 ` [PATCH RFC v2 16/16] media: add functions to allow creating interfaces Mauro Carvalho Chehab
2015-08-11 11:14   ` Hans Verkuil
2015-08-11 12:24     ` Mauro Carvalho Chehab
2015-08-11 12:26       ` Hans Verkuil
2015-08-07 15:23 ` [PATCH RFC v2 00/16] Changes on MC core due to MC workshop discussion Mauro Carvalho Chehab
     [not found] ` <cover.1439292977.git.mchehab@osg.samsung.com>
2015-08-11 12:09   ` [PATCH RFC v2 09/16] media: use media_graph_obj for link endpoints Mauro Carvalho Chehab
2015-08-11 12:09     ` Mauro Carvalho Chehab
2015-08-11 12:09     ` Mauro Carvalho Chehab
     [not found]     ` <cover.1439294756.git.mchehab@osg.samsung.com>
2015-08-11 12:09       ` [PATCH RFC v2 10/16] media: rename the function that create pad links Mauro Carvalho Chehab
2015-08-11 12:09         ` Mauro Carvalho Chehab
2015-08-11 12:09         ` Mauro Carvalho Chehab
2015-08-11 12:09         ` Mauro Carvalho Chehab
2015-08-11 12:25     ` [PATCH RFC v2 09/16] media: use media_graph_obj for link endpoints Hans Verkuil
2015-08-11 12:25       ` Hans Verkuil
2015-08-11 13:22       ` Mauro Carvalho Chehab
2015-08-11 13:22         ` Mauro Carvalho Chehab
2015-08-11 13:22         ` Mauro Carvalho Chehab
2015-08-11 13:32         ` Hans Verkuil
2015-08-11 13:32           ` Hans Verkuil
2015-08-11 13:32           ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55C9D344.8090905@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=boris.brezillon@free-electrons.com \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=mchehab@osg.samsung.com \
    --cc=prabhakar.csengg@gmail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=scott.jiang.linux@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.