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

[Andy Green <andy@warmcat.com>]

Johannes Berg wrote:
> On Tue, 2007-06-12 at 22:42 -0700, Michael Wu wrote:
>> 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..
> 
> I would as well, but as I outlined before we basically have four things
> that can happen
>  * we found a next item we understood
>  * we have a parser error
>  * we reached the end
>  * we found an item we can't support (and thus we don't support any
>    further ones either assuming we add support sequentially)
> 
> I would like to be able to distinguish all these cases, the previous
> version couldn't distinguish between the last two. However, I'm not
> convinced that error codes are reasonable for something that isn't an
> error (i.e. reaching the end)

Is it useful to report that there were items in the radiotap section
that were not illegal but were not comprehensible either?  Any app that
is written against the parser will be using the contempary list of
constants from ieee80211_radiotap.h, and anything added in there should
be added to radiotap.c.  Put another way you won't be in any better
position to understand or handle radiotap args that cfg80211 doesn't
understand than cfg80211 is itself.  Is there a case where you don't
want to ignore these unknown later entries that could appear?

This parser is used in userspace apps too, packetspammer, penumbrad,
aircrack and mdk, and hopefully others when injection becomes something
that happens out of the box.  The E* error constants are available in
userspace too.  Although you can make a case for either way probably
radiotap isn't so wonderfully grand that it deserves its own enum for
its results IMO.

-Andy