[PATCH] 3c59x: fix build failure on !CONFIG_PCI

[PATCH] 3c59x: fix build failure on !CONFIG_PCI

[Namhyung Kim]

VORTEX_PCI() could return NULL so it needs to be casted before
accessing any member of struct pci_dev. This fixes following
build failure. Likewise VORTEX_EISA() was changed also.

  CC [M]  drivers/net/3c59x.o
drivers/net/3c59x.c: In function 'acpi_set_WOL':
drivers/net/3c59x.c:3211:39: warning: dereferencing 'void *' pointer
drivers/net/3c59x.c:3211:39: error: request for member 'current_state' in something not a structure or union
make[3]: *** [drivers/net/3c59x.o] Error 1
make[2]: *** [drivers/net/3c59x.o] Error 2
make[1]: *** [sub-make] Error 2
make: *** [all] Error 2

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 drivers/net/3c59x.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
index e1da258..0a92436 100644
--- a/drivers/net/3c59x.c
+++ b/drivers/net/3c59x.c
@@ -699,7 +699,8 @@ DEFINE_WINDOW_IO(32)
 #define DEVICE_PCI(dev) NULL
 #endif
 
-#define VORTEX_PCI(vp) (((vp)->gendev) ? DEVICE_PCI((vp)->gendev) : NULL)
+#define VORTEX_PCI(vp)							\
+	((struct pci_dev *) (((vp)->gendev) ? DEVICE_PCI((vp)->gendev) : NULL))
 
 #ifdef CONFIG_EISA
 #define DEVICE_EISA(dev) (((dev)->bus == &eisa_bus_type) ? to_eisa_device((dev)) : NULL)
@@ -707,7 +708,8 @@ DEFINE_WINDOW_IO(32)
 #define DEVICE_EISA(dev) NULL
 #endif
 
-#define VORTEX_EISA(vp) (((vp)->gendev) ? DEVICE_EISA((vp)->gendev) : NULL)
+#define VORTEX_EISA(vp)							\
+	((struct eisa_device *) (((vp)->gendev) ? DEVICE_EISA((vp)->gendev) : NULL))
 
 /* The action to take with a media selection timer tick.
    Note that we deviate from the 3Com order by checking 10base2 before AUI.
-- 
1.7.0.4

Re: [PATCH] 3c59x: fix build failure on !CONFIG_PCI

[Randy Dunlap]

On Wed, 17 Nov 2010 00:27:51 +0900 Namhyung Kim wrote:

> VORTEX_PCI() could return NULL so it needs to be casted before
> accessing any member of struct pci_dev. This fixes following
> build failure. Likewise VORTEX_EISA() was changed also.
> 
>   CC [M]  drivers/net/3c59x.o
> drivers/net/3c59x.c: In function 'acpi_set_WOL':
> drivers/net/3c59x.c:3211:39: warning: dereferencing 'void *' pointer
> drivers/net/3c59x.c:3211:39: error: request for member 'current_state' in something not a structure or union
> make[3]: *** [drivers/net/3c59x.o] Error 1
> make[2]: *** [drivers/net/3c59x.o] Error 2
> make[1]: *** [sub-make] Error 2
> make: *** [all] Error 2
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  drivers/net/3c59x.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/3c59x.c b/drivers/net/3c59x.c
> index e1da258..0a92436 100644
> --- a/drivers/net/3c59x.c
> +++ b/drivers/net/3c59x.c
> @@ -699,7 +699,8 @@ DEFINE_WINDOW_IO(32)
>  #define DEVICE_PCI(dev) NULL
>  #endif
>  
> -#define VORTEX_PCI(vp) (((vp)->gendev) ? DEVICE_PCI((vp)->gendev) : NULL)
> +#define VORTEX_PCI(vp)							\
> +	((struct pci_dev *) (((vp)->gendev) ? DEVICE_PCI((vp)->gendev) : NULL))
>  
>  #ifdef CONFIG_EISA
>  #define DEVICE_EISA(dev) (((dev)->bus == &eisa_bus_type) ? to_eisa_device((dev)) : NULL)
> @@ -707,7 +708,8 @@ DEFINE_WINDOW_IO(32)
>  #define DEVICE_EISA(dev) NULL
>  #endif
>  
> -#define VORTEX_EISA(vp) (((vp)->gendev) ? DEVICE_EISA((vp)->gendev) : NULL)
> +#define VORTEX_EISA(vp)							\
> +	((struct eisa_device *) (((vp)->gendev) ? DEVICE_EISA((vp)->gendev) : NULL))
>  
>  /* The action to take with a media selection timer tick.
>     Note that we deviate from the 3Com order by checking 10base2 before AUI.
> -- 

Hi,

Interesting patch.  I have reported this build error and looked
into fixing it, but did not come up with this solution.

Looking at it more:  if CONFIG_PCI is not enabled, DEVICE_PCI() is NULL.
That makes VORTEX_PCI() (with or without your patch) have a value of NULL.

Is the line with the reported syntax error (3211) executed in
function acpi_set_WOL() ?  If so, let's assume that vp->enable_wol is true.
Then what happens on line 3211 (or 3213 after your patch)?

		if (VORTEX_PCI(vp)->current_state < PCI_D3hot)
			return;

or if I am really confuzed this morning, please tell me how it works.

thanks,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] 3c59x: fix build failure on !CONFIG_PCI

[Namhyung Kim]

2010-11-16 (화), 09:14 -0800, Randy Dunlap:
> Hi,
> 

Hi, Randy


> Interesting patch.  I have reported this build error and looked
> into fixing it, but did not come up with this solution.
> 
> Looking at it more:  if CONFIG_PCI is not enabled, DEVICE_PCI() is NULL.
> That makes VORTEX_PCI() (with or without your patch) have a value of NULL.
> 
> Is the line with the reported syntax error (3211) executed in
> function acpi_set_WOL() ?  If so, let's assume that vp->enable_wol is true.
> Then what happens on line 3211 (or 3213 after your patch)?
> 
> 		if (VORTEX_PCI(vp)->current_state < PCI_D3hot)
> 			return;
> 
> or if I am really confuzed this morning, please tell me how it works.
> 

At first glance, I've noticed that the code above could make a NULL
dereference so I added NULL check prior to the line.

But after reading more code I realized that other pci-functions called
in acpi_set_WOL() would not work with NULL pci_dev pointer also. And I
found all callers of the acpi_set_WOL() already checked NULL pointer
before the call. Finally I could remove the NULL check and leave the
code as is. That's how it works. :)

Thanks.

-- 
Regards,
Namhyung Kim

Re: [PATCH] 3c59x: fix build failure on !CONFIG_PCI

[Randy Dunlap]

On 11/16/10 18:22, Namhyung Kim wrote:
> 2010-11-16 (화), 09:14 -0800, Randy Dunlap:
>> Hi,
>>
> 
> Hi, Randy
> 
> 
>> Interesting patch.  I have reported this build error and looked
>> into fixing it, but did not come up with this solution.
>>
>> Looking at it more:  if CONFIG_PCI is not enabled, DEVICE_PCI() is NULL.
>> That makes VORTEX_PCI() (with or without your patch) have a value of NULL.
>>
>> Is the line with the reported syntax error (3211) executed in
>> function acpi_set_WOL() ?  If so, let's assume that vp->enable_wol is true.
>> Then what happens on line 3211 (or 3213 after your patch)?
>>
>> 		if (VORTEX_PCI(vp)->current_state < PCI_D3hot)
>> 			return;
>>
>> or if I am really confuzed this morning, please tell me how it works.
>>
> 
> At first glance, I've noticed that the code above could make a NULL
> dereference so I added NULL check prior to the line.
> 
> But after reading more code I realized that other pci-functions called
> in acpi_set_WOL() would not work with NULL pci_dev pointer also. And I
> found all callers of the acpi_set_WOL() already checked NULL pointer
> before the call. Finally I could remove the NULL check and leave the
> code as is. That's how it works. :)

I see.  and concur.  Thanks for the explanation.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] 3c59x: fix build failure on !CONFIG_PCI

[David Miller]

From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Wed, 17 Nov 2010 07:58:36 -0800

> On 11/16/10 18:22, Namhyung Kim wrote:
>> 2010-11-16 (화), 09:14 -0800, Randy Dunlap:
>>> Hi,
>>>
>> 
>> Hi, Randy
>> 
>> 
>>> Interesting patch.  I have reported this build error and looked
>>> into fixing it, but did not come up with this solution.
>>>
>>> Looking at it more:  if CONFIG_PCI is not enabled, DEVICE_PCI() is NULL.
>>> That makes VORTEX_PCI() (with or without your patch) have a value of NULL.
>>>
>>> Is the line with the reported syntax error (3211) executed in
>>> function acpi_set_WOL() ?  If so, let's assume that vp->enable_wol is true.
>>> Then what happens on line 3211 (or 3213 after your patch)?
>>>
>>> 		if (VORTEX_PCI(vp)->current_state < PCI_D3hot)
>>> 			return;
>>>
>>> or if I am really confuzed this morning, please tell me how it works.
>>>
>> 
>> At first glance, I've noticed that the code above could make a NULL
>> dereference so I added NULL check prior to the line.
>> 
>> But after reading more code I realized that other pci-functions called
>> in acpi_set_WOL() would not work with NULL pci_dev pointer also. And I
>> found all callers of the acpi_set_WOL() already checked NULL pointer
>> before the call. Finally I could remove the NULL check and leave the
>> code as is. That's how it works. :)
> 
> I see.  and concur.  Thanks for the explanation.

Looks good to me too, applied, thanks!