On Wed, Dec 8, 2010 at 1:15 AM, Guennadi Liakhovetski
<g.liakhovetski@xxxxxx> wrote:
> On Wed, 8 Dec 2010, David Cohen wrote:
>
>> OmniVision OV9640 driver wasn't deallocating properly its private data
>> as it was requesting the V4L2 subdev struct address instead of the priv
>> struct's one. This patch fixes such problem.
>>
>> Signed-off-by: David Cohen <dacohen@xxxxxxxxx>
>> ---
>> Âdrivers/media/video/ov9640.c | Â Â3 ++-
>> Â1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/video/ov9640.c b/drivers/media/video/ov9640.c
>> index 99e9e1d..1315696 100644
>> --- a/drivers/media/video/ov9640.c
>> +++ b/drivers/media/video/ov9640.c
>> @@ -791,7 +791,8 @@ static int ov9640_probe(struct i2c_client *client,
>>
>> Âstatic int ov9640_remove(struct i2c_client *client)
>> Â{
>> - Â Â struct ov9640_priv *priv = i2c_get_clientdata(client);
>> + Â Â struct v4l2_subdev *sd = i2c_get_clientdata(client);
>> + Â Â struct ov9640_priv *priv = container_of(sd, struct ov9640_priv, subdev);
>
> Ok, this doesn't crash, because subdev is currently the first member of
> struct ov9640_priv, so, this patch only fixes an "academic" problem. But
> even then it solves it incompletely:
>
> static int ov9640_video_probe(struct soc_camera_device *icd,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âstruct i2c_client *client)
> {
> Â Â Â Âstruct ov9640_priv *priv = i2c_get_clientdata(client);
True. I missed that one.
>
> So, sorry for changing my mind so quickly... We either need a complete
> patch, fixing both these locations, or we leave it as is until 2.6.38.
> TBH, I'd prefer the latter, what do you think? Interestingly, you also
> don't fix this location in v1 of your patches. So, I suggest, we make a
> proper fix and schedule it for 2.6.38.
IMO we can schedule it for 2.6.38 and release a proper fix. The first
patch is also very welcome as the driver is recovering priv data
through something like this:
static int ov9640_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct ov9640_priv *priv = container_of(i2c_get_clientdata(client),
struct ov9640_priv, subdev);
but i2c_get_clientdata(client) == sd, which is already an argument.
Br,
David
>
>> Â Â Â kfree(priv);
>> Â Â Â return 0;
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
>
--
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
