Wireshark-dev: Re: [Wireshark-dev] The ieee802.11 dissector is a steaming pile of ordure
On Thu, Sep 10, 2015 at 12:42 AM, Alexis La Goutte
<alexis.lagoutte@xxxxxxxxx> wrote:
> Hi Richard (and Bill)
>
> It is historic... at the first, there is only limited fixed field and tagged
> field...
> and on last 802.11 spec, there is > 100 tagged field...
>
> If you have a solution to avoid a big switch...
> i have already think to put each case on separate function... but for some
> case, there is only 5 lines of code...
I think the switch statement is OK, however, I would break out all the
open coded stuff into separate subroutines. I estimate that it would
reduce that subroutine to about 200-300 lines then.
I have vague thoughts about cleaning that dissector up, but I need to
find time. However, being aware of more problems makes the scope of
the effort clearer.
I also want to pass the phdr on to all the subdissectors so the
current crazy mechanism can be removed.
> Regards,
>
> On Wed, Sep 9, 2015 at 6:11 PM, Bill Meier <wmeier@xxxxxxxxxxx> wrote:
>>
>> On 9/9/2015 12:03 PM, Bill Meier wrote:
>>>
>>> On 9/9/2015 11:23 AM, Richard Sharpe wrote:
>>>>
>>>> Take a look at epan/dissectors/packet-ieee80211.c!
>>>>
>>>> Specifically, add_tagged_field.
>>>>
>>>> That function is approximately 2,300 lines long and it consists of one
>>>> big switch statement with every arm containing open-coded statements
>>>> to add things to the proto tree.
>>>>
>>>
>>> It's even worse:
>>>
>>> add_fixed_field() given a "fixed field number" does a linear search thru
>>> a (large) table to to find the number (and the associated function
>>> address) and then calls the function ...
>>>
>>> One side effect: there are functions which aren't used but since they're
>>> in the table, they're not flagged as unused by the compiler.
>>>
>>> In several cases there is (or was) duplicate code elsewhere doing a
>>> dissection similar to the unused "fixed field functions".
>>>
>>> (I was working to fix all this but got a bit bored because I had to
>>> spend time delving thru the 802211 spec trying to understand the code.
>>> I guess I should at least do that work (unless you have a broader
>>> solution in mind to handle both tagged and fixed fields ?)
>>>
>>
>>
>> Actually, I guess add_tagged_field and add_fixed_field are sort of doing
>> the same thing (just in different ways) with respect to the lookup.
>>
>>
>>
>>
>>
>> ___________________________________________________________________________
>> Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
>> Archives: https://www.wireshark.org/lists/wireshark-dev
>> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>> mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
>
>
>
> ___________________________________________________________________________
> Sent via: Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives: https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
> mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
--
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)
