On Fri, 2008-06-20 at 09:06 +0200, Holger Schurig wrote: > > Especially for the mesh_dev thing, you should probably have a > > static inline that returns priv->mesh_dev (mesh case) and NULL > > (non-mesh case) and rely on the compiler to elide as much code > > as possible. > > > > E.g. this: > > > +#ifdef CONFIG_LIBERTAS_MESH > > > if (priv->mesh_dev && (priv->mesh_connect_status == > > > LBS_CONNECTED)) netif_wake_queue(priv->mesh_dev); > > > +#endif > > > > wouldn't then need an ifdef. > > Ah, I thought that the compiler would barf about the > priv->mesh_connect_status, because this is no longer in > struct lbs_private. > > So I tried your suggestion and, yes, the compiler barfs: Yeah, of course, but that's actually the benefit of that method, the stuff in there is compiled. You already have a static inline for that (mesh_is_connected or something) though. As it stands, your patch really makes the driver hard to maintain, one will have to pretty much compile with and without mesh configured to test changes. IMHO. YMMV, and I don't really think my opinion for that driver should matter much :) johannes
Attachment:
signature.asc
Description: This is a digitally signed message part