On Monday 28 of January 2008 19:08:54 Randy Dunlap wrote:
> > +/* Number of bytes in NL packet header (can not do
>
> cannot
Fixed.
>
> > + * sizeof(nl_packet_header) since it's a bitfield) */
> > +#define NL_FOLLOWING_PACKET_HEADER_SIZE 1
> > +
> > +struct nl_first_paket_header {
>
> packet ?
Yes, packet of course.
>
> > +#if defined(__BIG_ENDIAN)
> > + unsigned char packet_rank:2;
> > + unsigned char address:3;
> > + unsigned char protocol:3;
> > +#else
> > + unsigned char protocol:3;
> > + unsigned char address:3;
> > + unsigned char packet_rank:2;
> > +#endif
>
> From C99 spec:
> "The order of allocation of bit-fields within a unit (high-order to
> low-order or low-order to high-order) is implementation-defined."
>
> so if the order/location of these bitfields is important (from one
> system to another), you should use bit masks instead of bitfields
> for them.
I've changed it to __BIG_ENDIAN_BITFIELDS, as suggested by Alexey. The order
is important. I'll add it to my todo to convert it to bitmasks.
> > +/*
> > + * @return 1 if something has been received from hw
>
> What's with the '@'?
Removed, and converted to plaintext as suggested by others.
> Need ":" and/or space between CARD_NAME and following string.
> (in several places)
Found a few of them and fixed.
> > +module_param(major, int, 0);
> > +module_param(ipwireless_debug, int, 0);
> > +module_param(ipwireless_loopback, int, 0);
> > +module_param(ipwireless_out_queue, int, 0);
> > +MODULE_PARM_DESC(major, "ttyIPWp major number [0]");
> > +MODULE_PARM_DESC(ipwireless_debug, "switch on debug messages [0]");
> > +MODULE_PARM_DESC(ipwireless_debug, "switch on loopback mode [0]");
> > +MODULE_PARM_DESC(ipwireless_debug, "set size of outgoing queue [1]");
>
> Will these parameters be documented anywhere?
The options are intended for debugging. Normal user does not need to tweak
them. Adjusted the description to reflect their debugging purpose.
> > +#ifdef IPWIRELESS_STATE_DEBUG
> > +int ipwireless_dump_network_state(char *p, struct ipw_network *network)
> > +{
> > + int idx = 0;
> > +
> > + idx += sprintf(p + idx, "debug: ppp_blocked=%d\n",
> > + network->ppp_blocked);
> > + idx += sprintf(p + idx, "debug: outgoing_packets_queued=%d\n",
> > + network->outgoing_packets_queued);
> > + idx += sprintf(p + idx, "debug: network.shutting_down=%d\n",
> > + network->shutting_down);
>
> check for overflow of 'p'?
Converted to one snprintf too.
> > +/* Syncronous start-messages */
>
> Synchronous
Fixed.
Updated patch will follow.
Thank you for comments.
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
