[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080728222925.GA21012@elte.hu>
Date: Tue, 29 Jul 2008 00:29:25 +0200
From: Ingo Molnar <mingo@...e.hu>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: torvalds@...ux-foundation.org, travis@....com,
rusty@...tcorp.com.au, linux-kernel@...r.kernel.org,
viro@...IV.linux.org.uk, dsterba@...e.cz, jkosina@...e.cz
Subject: Re: [build error] drivers/char/pcmcia/ipwireless/hardware.c:571:
error: invalid use of undefined type 'struct ipw_network'
* Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Tue, 29 Jul 2008 00:06:31 +0200
> Ingo Molnar <mingo@...e.hu> wrote:
>
> >
> > * Ingo Molnar <mingo@...e.hu> wrote:
> >
> > > drivers/char/pcmcia/ipwireless/hardware.c:571: error: invalid use of
> > > undefined type 'struct ipw_network'
> > >
> > > http://redhat.com/~mingo/misc/config-Mon_Jul_28_23_36_20_CEST_2008.bad
> > >
> > > probably due to a string of commits to that file from today.
> >
> > hm, has this version of the driver ever been built successfully? Because
> > struct ipw_network is defined in network.c [only], and then used in
> > hardware.c.
> >
> > It could be changed to void * if the structure wasnt relied on by:
> >
> > const int min_capacity =
> > ipwireless_ppp_mru(hw->network + 2);
> >
> > so changing it to void * would break this part of the code.
> >
> > furthermore, what does that "hw->network + 2" mean? It points into
> > la-la-land AFAICS, because it's initialized as:
> >
> > struct ipw_network *network =
> > kzalloc(sizeof(struct ipw_network), GFP_ATOMIC);
> >
> > and then written into hw->network via:
> >
> > ipwireless_associate_network(hw, network);
> >
> > it's getting late here, so i might be missing some really obvious
> > solution (and Jiri is asleep i suspect), so below is a temporary patch
> > that marks the driver CONFIG_BROKEN until this is resolved. This gets
> > allyesconfig going on x86.
>
> I suspect that this:
>
> --- a/drivers/char/pcmcia/ipwireless/hardware.c~a
> +++ a/drivers/char/pcmcia/ipwireless/hardware.c
> @@ -568,7 +568,7 @@ static struct ipw_rx_packet *pool_alloca
> list_del(&packet->queue);
> } else {
> const int min_capacity =
> - ipwireless_ppp_mru(hw->network + 2);
> + ipwireless_ppp_mru(hw->network) + 2;
> int new_capacity;
>
> spin_unlock_irqrestore(&hw->lock, flags);
> _
>
> was intended.
ah, indeed. I see, this came from the 12/12 patch:
- static int min_capacity = 256;
+ const int min_capacity =
+ ipwireless_ppp_mru(hw->network + 2);
which was probably a last-minute change. So instead of turning the
driver into something (while the author is sleeping) that was never
tested before, i'd rather suggest to revert commit
a01386924874c4d6d67f8a34e66f04452c2abb69, which seems a non-essential
optimization to the driver and which apparently was not build tested, so
quite likely not functionality tested either. It reverts cleanly here.
OTOH ... your change looks correct enough too.
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists