On Thu, Sep 10, 2015 at 12:42 AM, Alexis La Goutte <[email protected]> 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 <[email protected]> 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 <[email protected]> >> Archives: https://www.wireshark.org/lists/wireshark-dev >> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev >> mailto:[email protected]?subject=unsubscribe > > > > ___________________________________________________________________________ > Sent via: Wireshark-dev mailing list <[email protected]> > Archives: https://www.wireshark.org/lists/wireshark-dev > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev > mailto:[email protected]?subject=unsubscribe -- Regards, Richard Sharpe (何以解憂？唯有杜康。--曹操) ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
