On Friday 11 January 2008 17:40:04 Larry Finger wrote:
> Michael Buesch wrote:
> > This patch adds support for new firmware.
> > Please test this on old and new firmware.
>
> I have tested this patch with old firmware. It seems to work; however my testing is not complete as
> my computer has started hanging with the "Caps Lock" light flashing. The crash is not caused by this
> patch as it happened with 2.6.24-rc5, which has run for many days. I do have a suggestion for
> changing the patch (see below).
>
> > +static inline
> > +size_t b43_txhdr_size(struct b43_wldev *dev)
> > +{
> > + if (b43_is_old_txhdr_format(dev))
> > + return 100 + sizeof(struct b43_plcp_hdr6);
> > + return 104 + sizeof(struct b43_plcp_hdr6);
> > +}
>
> Why not eliminate most of the magic numbers in this part with
>
> size_t b43_txhdr_size(struct b43_wldev *dev)
> {
> if (b43_is_old_txhdr_format(dev))
> return sizeof(struct b43_txhdr) - 4;
> return sizeof(struct b43_txhdr);
> }
Because this is IMO as magic as the above.
The struct b43_txhdr is _not_ meant to be used as an object anymore,
as it now contains this union magic stuff. So we must only use it
as a pointer type. The sizeof, however, uses it as an object.
I'm perfectly fine with the hardcoded constants. And they really
are constants, as they are defined by the hard(firm)ware.
I think this all leads to the same issue as "Should we use
#defines for the PCI IDs in PCI ID tables?".
However, this code will go away in summer anyway. So it doesn't
really matter. It really is just a hack.
--
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
