From: Wolfram Sang <w.sang@pengutronix.de>
To: Jean Delvare <khali@linux-fr.org>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
Mark Brown <broonie@opensource.wolfsonmicro.com>,
kernel-janitors@vger.kernel.org, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org,
Mauro Carvalho Chehab <mchehab@infradead.org>,
linux-media@vger.kernel.org
Subject: Re: [PATCH 12/24] media/video: fix dangling pointers
Date: Tue, 23 Mar 2010 00:35:25 +0000 [thread overview]
Message-ID: <20100323003525.GC5762@pengutronix.de> (raw)
In-Reply-To: <20100322213358.31e50b3c@hyperion.delvare>
[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]
> > > Personally I'd much rather just not bother setting the driver data in
> > > the removal path, it seems unneeded. I had assumed that the subsystem
> > > code cared for some reason when I saw the patch series.
> >
> > Anyway, should this really be necessary, then for the media drivers this
> > should be done in v4l2_device_unregister_subdev() and not in every driver.
> >
> > But this just feels like an i2c core thing to me. After remove() is called
> > the core should just set the client data to NULL. If there are drivers that
> > rely on the current behavior, then those drivers should be reviewed first as
> > to the reason why they need it.
>
> I tend to agree, now.
>
> Wolfram, how do you feel about this? I feel a little sorry that I more
> or less encouraged you to submit this patch series, and now I get to
> agree with the objections which were raised against it.
Well, this is a valuable outcome in my book. Maybe seeing the actual amount of
modifications necessary for cleaning up clientdata helped the process. While
working on it, I also got the impression that it should be handled differently
in the future, of course. Although I was thinking of something different
('i2c_(allocate|free)_clientdata' as mentioned before), I prefer the above
proposal as it is most simple.
> My key motivation was that I wanted i2c_set_clientdata() to be called
> before kfree(). Now that everybody seems to agree that the latter
> belongs to the drivers while the former belongs to lower layers
> (i2c-core or even driver core), this is not going to happen. So I guess
> we want to remove calls to i2c_set_clientdata(NULL) from all drivers
> and have only one in i2c-core for now?
Fine with me. Let me know if I can assist you with the series.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <w.sang@pengutronix.de>
To: Jean Delvare <khali@linux-fr.org>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
Mark Brown <broonie@opensource.wolfsonmicro.com>,
kernel-janitors@vger.kernel.org, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org,
Mauro Carvalho Chehab <mchehab@infradead.org>,
linux-media@vger.kernel.org
Subject: Re: [PATCH 12/24] media/video: fix dangling pointers
Date: Tue, 23 Mar 2010 01:35:25 +0100 [thread overview]
Message-ID: <20100323003525.GC5762@pengutronix.de> (raw)
In-Reply-To: <20100322213358.31e50b3c@hyperion.delvare>
[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]
> > > Personally I'd much rather just not bother setting the driver data in
> > > the removal path, it seems unneeded. I had assumed that the subsystem
> > > code cared for some reason when I saw the patch series.
> >
> > Anyway, should this really be necessary, then for the media drivers this
> > should be done in v4l2_device_unregister_subdev() and not in every driver.
> >
> > But this just feels like an i2c core thing to me. After remove() is called
> > the core should just set the client data to NULL. If there are drivers that
> > rely on the current behavior, then those drivers should be reviewed first as
> > to the reason why they need it.
>
> I tend to agree, now.
>
> Wolfram, how do you feel about this? I feel a little sorry that I more
> or less encouraged you to submit this patch series, and now I get to
> agree with the objections which were raised against it.
Well, this is a valuable outcome in my book. Maybe seeing the actual amount of
modifications necessary for cleaning up clientdata helped the process. While
working on it, I also got the impression that it should be handled differently
in the future, of course. Although I was thinking of something different
('i2c_(allocate|free)_clientdata' as mentioned before), I prefer the above
proposal as it is most simple.
> My key motivation was that I wanted i2c_set_clientdata() to be called
> before kfree(). Now that everybody seems to agree that the latter
> belongs to the drivers while the former belongs to lower layers
> (i2c-core or even driver core), this is not going to happen. So I guess
> we want to remove calls to i2c_set_clientdata(NULL) from all drivers
> and have only one in i2c-core for now?
Fine with me. Let me know if I can assist you with the series.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
next prev parent reply other threads:[~2010-03-23 0:35 UTC|newest]
Thread overview: 171+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-20 14:12 i2c-drivers: Remove dangling pointers Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 04/24] gpio: fix " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-5-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-30 12:31 ` Wolfram Sang
2010-03-30 12:31 ` Wolfram Sang
2010-03-30 12:31 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 06/24] input/keyboard: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 19:20 ` Dmitry Torokhov
2010-03-20 19:20 ` Dmitry Torokhov
[not found] ` <20100320192007.GA28402-WlK9ik9hQGAhIp7JRqBPierSzoNAToWh@public.gmane.org>
2010-03-21 1:59 ` Wolfram Sang
2010-03-21 1:59 ` Wolfram Sang
2010-03-21 1:59 ` Wolfram Sang
2010-03-30 12:35 ` Wolfram Sang
2010-03-30 12:35 ` Wolfram Sang
2010-03-30 12:35 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 08/24] leds: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-9-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-30 12:36 ` Wolfram Sang
2010-03-30 12:36 ` Wolfram Sang
2010-03-30 12:36 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 09/24] macintosh: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 10/24] media/radio: " Wolfram Sang
[not found] ` <1269094385-16114-1-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-20 14:12 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a leak Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-2-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-20 14:57 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a Russell King
2010-03-20 14:57 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a leak Russell King
2010-03-20 14:57 ` Russell King
2010-03-20 14:57 ` Russell King
[not found] ` <20100320145729.GA5399-f404yB8NqCZvn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2010-03-20 15:31 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a Wolfram Sang
2010-03-20 15:31 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a leak Wolfram Sang
2010-03-20 15:31 ` Wolfram Sang
2010-03-20 15:31 ` Wolfram Sang
[not found] ` <20100320153146.GB5515-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-20 15:41 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a Russell King
2010-03-20 15:41 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a leak Russell King
2010-03-20 15:41 ` Russell King
2010-03-20 15:41 ` Russell King
[not found] ` <20100320154113.GB14470-f404yB8NqCZvn6HldHNs0ANdhmdF6hFW@public.gmane.org>
2010-03-21 8:30 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a David Woodhouse
2010-03-21 8:30 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a leak David Woodhouse
2010-03-21 8:30 ` David Woodhouse
2010-03-21 8:30 ` David Woodhouse
2010-03-30 12:26 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a Wolfram Sang
2010-03-30 12:26 ` [PATCH 01/24] mtd/maps/pismo: remove dangling pointer and a leak Wolfram Sang
2010-03-30 12:26 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 02/24] power/ds2782: really clear i2c_clientdata on exit Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-3-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-20 21:53 ` Ryan Mallon
2010-03-20 21:53 ` Ryan Mallon
2010-03-20 21:53 ` Ryan Mallon
2010-03-22 17:08 ` Anton Vorontsov
2010-03-22 17:08 ` Anton Vorontsov
2010-03-22 17:08 ` Anton Vorontsov
[not found] ` <20100322170810.GA30325-wnGakbxT3iijyJ0x5qLZdcN33GVbZNy3@public.gmane.org>
2010-03-30 12:28 ` Wolfram Sang
2010-03-30 12:28 ` Wolfram Sang
2010-03-30 12:28 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 03/24] usb/otg/isp1301_omap: Fix dangling pointer Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-4-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-20 20:09 ` Felipe Balbi
2010-03-20 20:09 ` Felipe Balbi
2010-03-20 20:09 ` Felipe Balbi
2010-03-30 12:30 ` Wolfram Sang
2010-03-30 12:30 ` Wolfram Sang
2010-03-30 12:30 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 05/24] hwmon: fix dangling pointers Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` [lm-sensors] " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-6-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-29 14:30 ` Jean Delvare
2010-03-29 14:30 ` Jean Delvare
2010-03-29 14:30 ` [lm-sensors] " Jean Delvare
2010-03-29 14:30 ` Jean Delvare
2010-03-20 14:12 ` [PATCH 07/24] input/touchscreen: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 11/24] media/radio/si470x: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 16/24] power: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 17/24] regulator: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-18-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-20 17:23 ` Mark Brown
2010-03-20 17:23 ` Mark Brown
2010-03-20 17:23 ` Mark Brown
2010-03-22 19:49 ` Liam Girdwood
2010-03-30 12:47 ` Wolfram Sang
2010-03-30 12:47 ` Wolfram Sang
2010-03-20 14:13 ` [PATCH 19/24] staging/dream: " Wolfram Sang
2010-03-20 14:13 ` Wolfram Sang
2010-03-20 14:13 ` [PATCH 22/24] staging/iio/light: " Wolfram Sang
2010-03-20 14:13 ` Wolfram Sang
2010-03-20 14:13 ` Wolfram Sang
2010-03-22 12:41 ` Jonathan Cameron
2010-04-05 22:59 ` i2c-drivers: Remove " Ben Dooks
2010-04-05 22:59 ` Ben Dooks
2010-04-05 22:59 ` Ben Dooks
[not found] ` <20100405225925.GC32401-elnMNo+KYs3pIgCt6eIbzw@public.gmane.org>
2010-04-06 0:06 ` Wolfram Sang
2010-04-06 0:06 ` Wolfram Sang
2010-04-06 0:06 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 12/24] media/video: fix " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 22:02 ` Hans Verkuil
2010-03-20 22:02 ` Hans Verkuil
[not found] ` <201003202302.49526.hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
2010-03-21 11:31 ` Wolfram Sang
2010-03-21 11:31 ` Wolfram Sang
2010-03-21 11:31 ` Wolfram Sang
2010-03-21 13:46 ` Jean Delvare
2010-03-21 13:46 ` Jean Delvare
2010-03-21 13:46 ` Jean Delvare
[not found] ` <20100321144655.4747fd2a-ig7AzVSIIG7kN2dkZ6Wm7A@public.gmane.org>
2010-03-21 14:14 ` Mark Brown
2010-03-21 14:14 ` Mark Brown
2010-03-21 14:14 ` Mark Brown
2010-03-21 16:09 ` Hans Verkuil
2010-03-21 16:09 ` Hans Verkuil
[not found] ` <201003211709.56319.hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
2010-03-22 20:33 ` Jean Delvare
2010-03-22 20:33 ` Jean Delvare
2010-03-22 20:33 ` Jean Delvare
2010-03-22 21:51 ` Mark Brown
2010-03-23 0:35 ` Wolfram Sang [this message]
2010-03-23 0:35 ` Wolfram Sang
2010-03-30 12:39 ` Wolfram Sang
2010-03-30 12:39 ` Wolfram Sang
2010-03-30 12:39 ` Wolfram Sang
2010-04-01 8:32 ` Hans Verkuil
2010-04-01 8:32 ` Hans Verkuil
2010-03-22 20:36 ` Jean Delvare
2010-03-22 20:36 ` Jean Delvare
2010-03-22 20:36 ` Jean Delvare
2010-03-20 14:12 ` [PATCH 13/24] mfd: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
2010-03-20 17:22 ` Mark Brown
[not found] ` <20100320172241.GD1549-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
2010-03-20 20:25 ` Jean Delvare
2010-03-20 20:25 ` Jean Delvare
2010-03-20 20:25 ` Jean Delvare
2010-03-21 2:09 ` Wolfram Sang
2010-03-21 2:09 ` Wolfram Sang
[not found] ` <1269094385-16114-14-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-25 10:41 ` Samuel Ortiz
2010-03-25 10:41 ` Samuel Ortiz
2010-03-25 10:41 ` Samuel Ortiz
[not found] ` <20100325104137.GA3720-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
2010-03-30 12:41 ` Wolfram Sang
2010-03-30 12:41 ` Wolfram Sang
2010-03-30 12:41 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 14/24] misc: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-15-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-30 12:44 ` Wolfram Sang
2010-03-30 12:44 ` Wolfram Sang
2010-03-30 12:44 ` Wolfram Sang
2010-03-20 14:12 ` [PATCH 15/24] misc/eeprom: " Wolfram Sang
2010-03-20 14:12 ` [PATCH 18/24] rtc: " Wolfram Sang
2010-03-20 14:12 ` Wolfram Sang
[not found] ` <1269094385-16114-19-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-30 12:49 ` Wolfram Sang
2010-03-30 12:49 ` Wolfram Sang
2010-03-30 12:49 ` Wolfram Sang
2010-03-20 14:13 ` [PATCH 20/24] staging/go7007: " Wolfram Sang
2010-03-20 14:13 ` [PATCH 21/24] staging/iio/adc: " Wolfram Sang
2010-03-20 14:13 ` Wolfram Sang
[not found] ` <1269094385-16114-22-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-03-22 12:41 ` Jonathan Cameron
2010-03-22 12:41 ` Jonathan Cameron
2010-03-22 12:41 ` Jonathan Cameron
2010-03-20 14:13 ` [PATCH 23/24] video/matrox: " Wolfram Sang
2010-03-20 14:13 ` Wolfram Sang
[not found] ` <1269094385-16114-24-git-send-email-w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2010-04-05 14:04 ` James Simmons
2010-04-05 14:04 ` James Simmons
2010-04-05 14:04 ` James Simmons
2010-03-20 14:13 ` [PATCH 24/24] w1/masters: " Wolfram Sang
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=20100323003525.GC5762@pengutronix.de \
--to=w.sang@pengutronix.de \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=hverkuil@xs4all.nl \
--cc=kernel-janitors@vger.kernel.org \
--cc=khali@linux-fr.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@infradead.org \
/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.