Re: [PATCH] four warnings and a patch

Eli Carter (eli.carter@inet.com)
Thu, 28 Oct 1999 17:25:52 -0500


Dennis Hou wrote:
>
> On Thu, 28 Oct 1999, Rik van Riel wrote:
>
> > On Thu, 28 Oct 1999, Dennis Hou wrote:
> >
> > > Can somebody more knowledgable about the above please get these warnings
> > > out? After all, perfect compilation is the first step to a Good kernel.

First, I'm not more knowledgeable... but try this--assume that there is
a reason the variable is there. Then find out what that is. Most of
the time, there is a reason. Assume the programmer knows what he was
doing... C does...

> >
> > Basically, your patches remove variables that are not referenced
> > on UP systems. This will kill any SMP compilation (let alone boot).
> > Let's just hope Linus doesn't die laughing.
>
> What are you talking about? There is only one line in the patch:
>
> - struct ei_device *ei_local = (struct ei_device *) dev->priv;
>

The problem lies in the macros later in the function.
E8390_CMD, EN0_TCNTLO, EN0_TCNTHI, EN0_TPSR all use the macro EI_SHIFT.
EI_SHIFT has 2 possible definitions.
>From 8390.h:
#if defined(CONFIG_MAC) || defined(CONFIG_AMIGA_PCMCIA) || \
defined(CONFIG_ARIADNE2) || defined(CONFIG_ARIADNE2_MODULE)
#define EI_SHIFT(x) (ei_local->reg_offset[x])
#else
#define EI_SHIFT(x) (x)
#endif

Therefore, if any of those config options are used, the variable _is_
used.

I think the Right Thing(TM) would be to do something like one of these
solutions:
Solution 1
in 8390.c:
#if defined(CONFIG_MAC) || defined(CONFIG_AMIGA_PCMCIA) || \
defined(CONFIG_ARIADNE2) || defined(CONFIG_ARIADNE2_MODULE)
struct ei_device *ei_local = (struct ei_device *) dev->priv;
#endif

Solution 2
in 8390.h:
#if defined(CONFIG_MAC) || defined(CONFIG_AMIGA_PCMCIA) || \
defined(CONFIG_ARIADNE2) || defined(CONFIG_ARIADNE2_MODULE)
#define EI_SHIFT(x) (ei_local->reg_offset[x])
#define NEED_EI_LOCAL
#else
#define EI_SHIFT(x) (x)
#endif

and in 8390.c:
#ifdef NEED_EI_LOCAL
struct ei_device *ei_local = (struct ei_device *) dev->priv;
#endif
Pros: the dependency is automatically propogated to the variable
declaration. Cons: introduces another #define.

Which of these is prefered, I'm not sure... can anyone enlighten me?

A couple more comments:
We are seeing lots of posts "fixed unused variable warnings". We need
to add to the FAQ some comments directed to this. Also, when these come
up, we need to actually fix them... make the declarations dependant upon
the same things that the use of said variable depends.

Eli Carter
#include<std_disclaimer.h>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/