From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from lists.s-osg.org ([54.187.51.154]:38603 "EHLO lists.s-osg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754325AbbBBNgT (ORCPT ); Mon, 2 Feb 2015 08:36:19 -0500 Date: Mon, 2 Feb 2015 11:36:13 -0200 From: Mauro Carvalho Chehab To: Lars-Peter Clausen Cc: Hans Verkuil , Sergei Shtylyov , Vladimir Barinov , Richard =?UTF-8?B?UsO2amZvcnM=?= , Federico Vaga , linux-media@vger.kernel.org Subject: Re: [PATCH v2 03/15] [media] adv7180: Use inline function instead of macro Message-ID: <20150202113613.07673af0@recife.lan> In-Reply-To: <1422028354-31891-4-git-send-email-lars@metafoo.de> References: <1422028354-31891-1-git-send-email-lars@metafoo.de> <1422028354-31891-4-git-send-email-lars@metafoo.de> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Em Fri, 23 Jan 2015 16:52:22 +0100 Lars-Peter Clausen escreveu: > Use a inline function instead of a macro for the container_of helper for > getting the driver's state struct from a control. A inline function has the > advantage that it is more typesafe and nicer in general. I don't see any advantage on this. See: container_of is already a macro, and it is written in a way that, if you use it with inconsistent values, the compilation will break. Also, there's the risk that, for whatever reason, gcc to decide to not inline this. So, this doesn't sound a good idea. Regards, Mauro > > Signed-off-by: Lars-Peter Clausen > Acked-by: Hans Verkuil > --- > drivers/media/i2c/adv7180.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c > index f424a4d..f2508abe 100644 > --- a/drivers/media/i2c/adv7180.c > +++ b/drivers/media/i2c/adv7180.c > @@ -130,9 +130,11 @@ struct adv7180_state { > bool powered; > u8 input; > }; > -#define to_adv7180_sd(_ctrl) (&container_of(_ctrl->handler, \ > - struct adv7180_state, \ > - ctrl_hdl)->sd) > + > +static struct adv7180_state *ctrl_to_adv7180(struct v4l2_ctrl *ctrl) > +{ > + return container_of(ctrl->handler, struct adv7180_state, ctrl_hdl); > +} > > static v4l2_std_id adv7180_std_to_v4l2(u8 status1) > { > @@ -345,9 +347,8 @@ static int adv7180_s_power(struct v4l2_subdev *sd, int on) > > static int adv7180_s_ctrl(struct v4l2_ctrl *ctrl) > { > - struct v4l2_subdev *sd = to_adv7180_sd(ctrl); > - struct adv7180_state *state = to_state(sd); > - struct i2c_client *client = v4l2_get_subdevdata(sd); > + struct adv7180_state *state = ctrl_to_adv7180(ctrl); > + struct i2c_client *client = v4l2_get_subdevdata(&state->sd); > int ret = mutex_lock_interruptible(&state->mutex); > int val; >