On Monday 11 June 2007 08:21, andy@xxxxxxxxxxx wrote:
> +__ieee80211_convert_radiotap_to_control_and_remove(
This is kinda long.. how about ieee80211_parse_radiotap instead? Function
names aren't a substitute for comments. ;)
> + struct ieee80211_hw_mode *hw_mode=tx->local->hw.conf.mode;
Add a space before and after the '='. Also, mode is more common than hw_mode
as a variable name for struct ieee80211_hw_mode.
> +
> + /* this can fail some sanity checks, drop packet if it does so */
> +
> + if (ieee80211_radiotap_iterator_init(&iterator, rthdr, skb->len) < 0)
> + return TXRX_DROP;
> +
> + /*
> + * default control situation for all injected packets
> + * FIXME: this does not suit all usage cases, expand to allow control
> + */
> +
> + control->retry_limit = 1; /* no retry */
> + control->key_idx = -1; /* no encryption key */
> + control->flags &= ~(IEEE80211_TXCTL_USE_RTS_CTS |
> + IEEE80211_TXCTL_USE_CTS_PROTECT);
> + control->flags |= IEEE80211_TXCTL_DO_NOT_ENCRYPT |
> + IEEE80211_TXCTL_NO_ACK;
> + control->antenna_sel_tx = 0; /* default to default antenna */
> +
> + /*
> + * for every radiotap entry that is present (returns -ve on end or
-ve?
> + * on error)
> + */
> +
> + while (ieee80211_radiotap_iterator_next(&iterator) >= 0) {
> + int i, target_rate;
> +
> + /* see if this argument is something we can use */
> +
> + switch (iterator.this_arg_index) {
> +
An empty line after a switch statement doesn't seem right.
I think the rest of the patch is okay. Just keep in mind the other style
comments I made in the other patch.
Thanks,
-Michael Wu
Attachment:
pgpO68P3Adtds.pgp
Description: PGP signature
