Wireshark · Wireshark-dev: Re: [Wireshark-dev] range_string checking

[Martin Mathieson <martin.r.mathieson@xxxxxxxxxxxxxx>]

In the previous message I said "  I suspect having a more complicated test probably find many more issues."  I meant to say that I doubted it would.

Have uploaded https://code.wireshark.org/review/#/c/36708/ for the remaining issues and the checking code itself.  Only spec I couldn't find for was ISUP - if someone who does have the spec could look it up that'd be great.

Thanks,

Martin

On Sat, Apr 4, 2020 at 9:24 PM Martin Mathieson <martin.r.mathieson@xxxxxxxxxxxxxx> wrote:

Yes, I'm sure I've seen an example where there is a catch-all at the end of each of a number of ranges, then a catch-all covering all ranges after that.

I am still only complaining if a later item is completely hidden by a single, earlier one.  If I understand what you said, I suppose I could check whether collectively all of the earlier items hide a later one. I suspect having a more complicated test probably find many more issues.

My plan is to fix the remaining handful of issues and check it in as it is, then I'll experiment with seeing if I can find any others.

The only other automated check along these lines I tried recently was for enumerated preferences (i.e. looking for duplicated IDs or strings), but it sadly(?) didn't find anything...

Martin

On Sat, Apr 4, 2020 at 2:53 PM Jaap Keuter <jaap.keuter@xxxxxxxxx> wrote:

On 2 Apr 2020, at 23:08, Martin Mathieson via Wireshark-dev <wireshark-dev@xxxxxxxxxxxxx> wrote:

It is common to have a 'catch-all' case for parts or all of the range, which is Ok if it comes after more specific entries.  I'm wondering if its worth complaining if *part* of an entry is hidden by an earlier one?  Current output from master is as below.  I will try to fix them up where I can access the relevant specs, but wanted to check my understanding of how they work and how fussy we should be?  I will most likely update README.dissector to make sure it is clear how it is evaluated in order.

Cool stuff. 

I can definitely see use for catch-all-in-certain-range, opposite of filling every gap with their specifics, which is maintenance heavy. This matches the val_to_string() default string used when no match is found, but then in a higher dimension. I would say let the ranges decide, if their union is the same as the catch-all then it’s okay, otherwise mark it.

just my €0.02

Jaap

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe