On Apr 4, 2014, at 4:04 PM, Guy Harris <guy@xxxxxxxxxxxx> 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