[PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

[PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

[Ezequiel Garcia]

Hi Mauro,

This patchset cleans the i2c part of some drivers.
This issue was recently reported by Dan Carpenter [1],
and revealed wrong (and harmless) usage of struct i2c_algo_bit.

Also, I properly assigned bus->i2c_rc (return code variable) and
replaced struct memcpy with struct assignment.
The latter is, in my opinion, a much safer way for struct filling
and I'm not aware of any drawbacks.

The patches are based on today's linux-next; I hope this is okey.
As I don't own any of these devices, I can't test the changes beyond
compilation.

Ezequiel Garcia (12):
  cx25821: Replace struct memcpy with struct assignment
  cx25821: Remove useless struct i2c_algo_bit_data usage
  cx25821: Use i2c_rc properly to store i2c register status
  cx231xx: Replace struct memcpy with struct assignment
  cx231xx: Remove useless struct i2c_algo_bit_data usage
  cx231xx: Use i2c_rc properly to store i2c register status
  cx23885: Replace struct memcpy with struct assignment
  cx23885: Remove useless struct i2c_algo_bit_data
  cx23885: Use i2c_rc properly to store i2c register status
  saa7164: Replace struct memcpy with struct assignment
  saa7164: Remove useless struct i2c_algo_bit_data
  saa7164: Use i2c_rc properly to store i2c register status

 drivers/media/video/cx231xx/cx231xx-i2c.c |   10 +++-------
 drivers/media/video/cx231xx/cx231xx.h     |    2 --
 drivers/media/video/cx23885/cx23885-i2c.c |   12 +++---------
 drivers/media/video/cx23885/cx23885.h     |    2 --
 drivers/media/video/cx25821/cx25821-i2c.c |   12 +++---------
 drivers/media/video/cx25821/cx25821.h     |    2 --
 drivers/media/video/saa7164/saa7164-i2c.c |   13 +++----------
 drivers/media/video/saa7164/saa7164.h     |    2 --
 8 files changed, 12 insertions(+), 43 deletions(-)

Thanks,
Ezequiel.

[1] http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/49553

RE: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

[Palash Bandyopadhyay]

Thanks Ezequiel and Dan. The changes look ok. I'll have someone check out the changes on the CX devices.

Rgds,
Palash

-----Original Message-----
From: Ezequiel Garcia [mailto:elezegarcia@gmail.com] 
Sent: Monday, June 18, 2012 12:23 PM
To: Mauro Carvalho Chehab
Cc: linux-media; Dan Carpenter; Palash Bandyopadhyay; stoth@kernellabs.com
Subject: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

Hi Mauro,

This patchset cleans the i2c part of some drivers.
This issue was recently reported by Dan Carpenter [1], and revealed wrong (and harmless) usage of struct i2c_algo_bit.

Also, I properly assigned bus->i2c_rc (return code variable) and replaced struct memcpy with struct assignment.
The latter is, in my opinion, a much safer way for struct filling and I'm not aware of any drawbacks.

The patches are based on today's linux-next; I hope this is okey.
As I don't own any of these devices, I can't test the changes beyond compilation.

Ezequiel Garcia (12):
  cx25821: Replace struct memcpy with struct assignment
  cx25821: Remove useless struct i2c_algo_bit_data usage
  cx25821: Use i2c_rc properly to store i2c register status
  cx231xx: Replace struct memcpy with struct assignment
  cx231xx: Remove useless struct i2c_algo_bit_data usage
  cx231xx: Use i2c_rc properly to store i2c register status
  cx23885: Replace struct memcpy with struct assignment
  cx23885: Remove useless struct i2c_algo_bit_data
  cx23885: Use i2c_rc properly to store i2c register status
  saa7164: Replace struct memcpy with struct assignment
  saa7164: Remove useless struct i2c_algo_bit_data
  saa7164: Use i2c_rc properly to store i2c register status

 drivers/media/video/cx231xx/cx231xx-i2c.c |   10 +++-------
 drivers/media/video/cx231xx/cx231xx.h     |    2 --
 drivers/media/video/cx23885/cx23885-i2c.c |   12 +++---------
 drivers/media/video/cx23885/cx23885.h     |    2 --
 drivers/media/video/cx25821/cx25821-i2c.c |   12 +++---------
 drivers/media/video/cx25821/cx25821.h     |    2 --
 drivers/media/video/saa7164/saa7164-i2c.c |   13 +++----------
 drivers/media/video/saa7164/saa7164.h     |    2 --
 8 files changed, 12 insertions(+), 43 deletions(-)

Thanks,
Ezequiel.

[1] http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/49553

Conexant E-mail Firewall (Conexant.Com) made the following annotations
---------------------------------------------------------------------
********************** Legal Disclaimer **************************** 

"This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you." 

********************************************************************** 

---------------------------------------------------------------------

Re: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

[Ezequiel Garcia]

Hi Palash,

On Mon, Jun 18, 2012 at 5:30 PM, Palash Bandyopadhyay
<Palash.Bandyopadhyay@conexant.com> wrote:
> Thanks Ezequiel and Dan. The changes look ok. I'll have someone check out the changes on the CX devices.
>

If you can have them tested, I think it would be great.

Thanks for reviewing,
Ezequiel.

Re: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

[Dan Carpenter]

On Mon, Jun 18, 2012 at 04:23:14PM -0300, Ezequiel Garcia wrote:
> Hi Mauro,
> 
> This patchset cleans the i2c part of some drivers.
> This issue was recently reported by Dan Carpenter [1],
> and revealed wrong (and harmless) usage of struct i2c_algo_bit.
> 

How is this harmless?  We are setting the function pointers to
something completely bogus.  It seems like a bad thing.

regards,
dan carpenter

Re: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

[Ezequiel Garcia]

On Mon, Jun 18, 2012 at 5:56 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Mon, Jun 18, 2012 at 04:23:14PM -0300, Ezequiel Garcia wrote:
>> Hi Mauro,
>>
>> This patchset cleans the i2c part of some drivers.
>> This issue was recently reported by Dan Carpenter [1],
>> and revealed wrong (and harmless) usage of struct i2c_algo_bit.
>>
>
> How is this harmless?  We are setting the function pointers to
> something completely bogus.  It seems like a bad thing.
>

You're right, but that wrongly assigned struct  algo_bit_data is never
*ever* used,
since it is not registered.

So, I meant it was harmless in that way, perhaps it wasn't the right term.

Of course, I can be wrong.
Ezequiel.

Re: [PATCH 0/12] struct i2c_algo_bit_data cleanup on several drivers

[Dan Carpenter]

On Mon, Jun 18, 2012 at 06:00:52PM -0300, Ezequiel Garcia wrote:
> On Mon, Jun 18, 2012 at 5:56 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > On Mon, Jun 18, 2012 at 04:23:14PM -0300, Ezequiel Garcia wrote:
> >> Hi Mauro,
> >>
> >> This patchset cleans the i2c part of some drivers.
> >> This issue was recently reported by Dan Carpenter [1],
> >> and revealed wrong (and harmless) usage of struct i2c_algo_bit.
> >>
> >
> > How is this harmless?  We are setting the function pointers to
> > something completely bogus.  It seems like a bad thing.
> >
> 
> You're right, but that wrongly assigned struct  algo_bit_data is never
> *ever* used,
> since it is not registered.
> 
> So, I meant it was harmless in that way, perhaps it wasn't the right term.

No no.  I didn't realize these files didn't call i2c_add_bus().
Harmless is the right term.

regards,
dan carpenter

[PATCH v2 0/9] struct i2c_algo_bit_data cleanup on several drivers

[Ezequiel Garcia]

Hi Mauro,

This patchset cleans the i2c part of some drivers.
This issue was recently reported by Dan Carpenter [1],
and revealed wrong (and harmless) usage of struct i2c_algo_bit_data.

This series consist in two kinds of patches:
 - remove one unused function
 - remove unused field i2c_algo
 - replace struct memcpy with struct assignment

This last change is, in my opinion, a much safer way for struct filling
and (in this case) I'm not aware of any change in functionality.

Also I've dropped i2c_rc cleaning entirely. I'm still working on these
and they are not related to this patchset.

As I don't own any of these devices, I can't test the changes beyond
compilation.

Changes from v1:
 - Drop i2c_rc clean patches
 - Verbose commit messages

Ezequiel Garcia (9):
 cx25821: Replace struct memcpy with struct assignment
 cx231xx: Replace struct memcpy with struct assignment
 cx23885: Replace struct memcpy with struct assignment
 saa7164: Replace struct memcpy with struct assignment
 saa7164: Remove unused saa7164_call_i2c_clients()
 cx25821: Remove useless struct i2c_algo_bit_data
 cx231xx: Remove useless struct i2c_algo_bit_data
 cx23885: Remove useless struct i2c_algo_bit_data
 saa7164: Remove useless struct i2c_algo_bit_data

 drivers/media/video/cx231xx/cx231xx-i2c.c |    8 ++------
 drivers/media/video/cx231xx/cx231xx.h     |    2 --
 drivers/media/video/cx23885/cx23885-i2c.c |   10 ++--------
 drivers/media/video/cx23885/cx23885.h     |    2 --
 drivers/media/video/cx25821/cx25821-i2c.c |   10 ++--------
 drivers/media/video/cx25821/cx25821.h     |    2 --
 drivers/media/video/saa7164/saa7164-i2c.c |   20 ++------------------
 drivers/media/video/saa7164/saa7164.h     |    2 --
 8 files changed, 8 insertions(+), 48 deletions(-)

Thanks,
Ezequiel.

[1] http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/49553