Re: [PATCH Try#11 3/4] cfg80211: Radiotap parser

[Michael Wu <flamingice@sourmilk.net>]

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

On Tuesday 12 June 2007 06:04, andy@warmcat.com wrote:
> +typedef enum  {
> +       RADIOTAP_PARSER_OK = 0,
> +       RADIOTAP_PARSER_DONE,
> +       RADIOTAP_PARSER_INVALID
> +} ieee80211_radiotap_parser_retcode_t;
Yuck. I much prefer the standard error codes used in the previous version..

> +extern ieee80211_radiotap_parser_retcode_t ieee80211_radiotap_iterator_init(
> +   struct ieee80211_radiotap_iterator *iterator,
> +   struct ieee80211_radiotap_header *radiotap_header,
> +   int max_length
> +);
Move the end of the parenthesis up a line to max_length.

> +ieee80211_radiotap_parser_retcode_t ieee80211_radiotap_iterator_init(
> +    struct ieee80211_radiotap_iterator * iterator,
                                           ^

> +    struct ieee80211_radiotap_header * radiotap_header,
                                         ^

> +	/* find payload start allowing for extended bitmap(s) */
> +
> +	if (unlikely(iterator->bitmap_shifter &
> +		     IEEE80211_RADIOTAP_PRESENT_EXTEND_MASK)) {
> +		while (le32_to_cpu(*((u32 *)iterator->arg)) &
                                    ^                    ^

> +ieee80211_radiotap_parser_retcode_t ieee80211_radiotap_iterator_next(
> +    struct ieee80211_radiotap_iterator * iterator)
                                           ^

> +		if (((ulong)iterator->arg - (ulong)iterator->rtheader) &
> +		    ((rt_sizes[iterator->arg_index] >> 4) - 1))
> +			iterator->arg_index +=
> +				(rt_sizes[iterator->arg_index] >> 4) -
> +				((((int)iterator->arg) -
> +				 ((int)iterator->rtheader)) &
> +				 ((rt_sizes[iterator->arg_index] >> 4) - 1));
> +
This part would probably look better if you stored:

((ulong)iterator->arg - (ulong)iterator->rtheader) & ((rt_sizes[iterator->arg_index] >> 4) - 1)

into a temporary variable.

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]