linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] e1000e: Use sizeof struct rather than pointer in e1000_get_eeprom()
@ 2009-11-21 19:26 Roel Kluin
  2009-11-22 14:31 ` Roel Kluin
  0 siblings, 1 reply; 3+ messages in thread
From: Roel Kluin @ 2009-11-21 19:26 UTC (permalink / raw)
  To: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, PJ Waskiewicz,
	John Ronciak, e1000-devel, Andrew Morton, LKML

The sizeof the struct should be used rather than of the pointer

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
 drivers/net/e1000e/ethtool.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Unless I am mistaken?

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 1bf4d2a..13c658e 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -508,7 +508,7 @@ static int e1000_get_eeprom(struct net_device *netdev,
 
 	if (ret_val) {
 		/* a read error occurred, throw away the result */
-		memset(eeprom_buff, 0xff, sizeof(eeprom_buff));
+		memset(eeprom_buff, 0xff, sizeof(*eeprom_buff));
 	} else {
 		/* Device's eeprom is always little-endian, word addressable */
 		for (i = 0; i < last_word - first_word + 1; i++)

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] e1000e: Use sizeof struct rather than pointer in e1000_get_eeprom()
  2009-11-21 19:26 [PATCH] e1000e: Use sizeof struct rather than pointer in e1000_get_eeprom() Roel Kluin
@ 2009-11-22 14:31 ` Roel Kluin
  2009-11-23 19:39   ` Jeff Kirsher
  0 siblings, 1 reply; 3+ messages in thread
From: Roel Kluin @ 2009-11-22 14:31 UTC (permalink / raw)
  To: Roel Kluin
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, PJ Waskiewicz,
	John Ronciak, e1000-devel, Andrew Morton, LKML

Don't use the sizeof the pointer to clear the result

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
 drivers/net/e1000e/ethtool.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

> -		memset(eeprom_buff, 0xff, sizeof(eeprom_buff));
> +		memset(eeprom_buff, 0xff, sizeof(*eeprom_buff));

No it's probably still not correct, eeprom_buff was kmalloc'd with:

eeprom_buff = kmalloc(sizeof(u16) *
                        (last_word - first_word + 1), GFP_KERNEL);

How about:

diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 1bf4d2a..5b276c0 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -508,7 +508,8 @@ static int e1000_get_eeprom(struct net_device *netdev,
 
 	if (ret_val) {
 		/* a read error occurred, throw away the result */
-		memset(eeprom_buff, 0xff, sizeof(eeprom_buff));
+		memset(eeprom_buff, 0xff, sizeof(u16) *
+				(last_word - first_word + 1));
 	} else {
 		/* Device's eeprom is always little-endian, word addressable */
 		for (i = 0; i < last_word - first_word + 1; i++)

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] e1000e: Use sizeof struct rather than pointer in  e1000_get_eeprom()
  2009-11-22 14:31 ` Roel Kluin
@ 2009-11-23 19:39   ` Jeff Kirsher
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Kirsher @ 2009-11-23 19:39 UTC (permalink / raw)
  To: Roel Kluin
  Cc: Jesse Brandeburg, Bruce Allan, PJ Waskiewicz, John Ronciak,
	e1000-devel, Andrew Morton, LKML

On Sun, Nov 22, 2009 at 06:31, Roel Kluin <roel.kluin@gmail.com> wrote:
> Don't use the sizeof the pointer to clear the result
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
>  drivers/net/e1000e/ethtool.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
>> -             memset(eeprom_buff, 0xff, sizeof(eeprom_buff));
>> +             memset(eeprom_buff, 0xff, sizeof(*eeprom_buff));
>
> No it's probably still not correct, eeprom_buff was kmalloc'd with:
>
> eeprom_buff = kmalloc(sizeof(u16) *
>                        (last_word - first_word + 1), GFP_KERNEL);
>
> How about:
>
> diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
> index 1bf4d2a..5b276c0 100644
> --- a/drivers/net/e1000e/ethtool.c
> +++ b/drivers/net/e1000e/ethtool.c
> @@ -508,7 +508,8 @@ static int e1000_get_eeprom(struct net_device *netdev,
>
>        if (ret_val) {
>                /* a read error occurred, throw away the result */
> -               memset(eeprom_buff, 0xff, sizeof(eeprom_buff));
> +               memset(eeprom_buff, 0xff, sizeof(u16) *
> +                               (last_word - first_word + 1));
>        } else {
>                /* Device's eeprom is always little-endian, word addressable */
>                for (i = 0; i < last_word - first_word + 1; i++)
> --

Thanks Roel.  I will add this to my tree for testing and review.

-- 
Cheers,
Jeff

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-11-23 19:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-21 19:26 [PATCH] e1000e: Use sizeof struct rather than pointer in e1000_get_eeprom() Roel Kluin
2009-11-22 14:31 ` Roel Kluin
2009-11-23 19:39   ` Jeff Kirsher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).