Wireshark mailing list archives

Re: No tvb_get for string-encoded numbers?


From: Hadriel Kaplan <hadriel.kaplan () oracle com>
Date: Sat, 5 Apr 2014 14:18:27 -0400


On Apr 4, 2014, at 4:04 PM, Guy Harris <guy () alum mit edu> wrote:

Likewise, it’s not clear if there’s a way to define a protocol field that is encoded as a string in the packet but 
is internally a uint8/16/32/64 (e.g., for filtering purposes, val_string lookup, etc.). For example such that 
proto_tree_add_item() would work. Instead, it seems some dissectors use the returned strtol/atoi to then add the 
field to the tree as a true uint type, or add it as a FT_STRING field type.

One advantage of that is that, if the routine to fetch the value also adds an item to the protocol tree, it could, in 
the cases where the value is invalid, also add an expert item indicating that the value isn't valid.

And I'd like to see proto_tree_add_XXX_item() routines that add an item with a particular type *and* take a pointer 
argument and return the value for the item through that pointer; that could replace
[snip]

Regarding the above, when proto_tree_add_XXX_item() is called and there is no tree, should the routine throw an error 
if the value can't be decoded from the string? (e.g., if the string isn't a valid number or is out of the valid range 
for the number type)

And if there *is* a tree, should it just add an expert item of the error, or should it also throw an error?

In other words, should dissection continue in such cases, or not?

Currently most of the consumers of such a routine don't seem to do any error checking, so they would continue 
dissection even if their call to strtol/atoi/whatever failed. (they get back the number 0 or MAX/MIN and continue on 
their merry way)

So if I make this new set of routines throw an error in such cases, I'd be changing how current dissectors behave. My 
inclination is to not throw an error, but just let things continue, under the principle of letting the dissector 
dissect as much as it can. (unless the dissector explicitly checks for an error because it has to be valid to continue, 
but that will obviously continue to be supported as well)

-hadriel

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


Current thread: