Comment # 24
on bug 9323
from Evan Huus
(In reply to comment #22)
> (In reply to comment #21)
> > There are cases where the caller can't assume NUL termination without doing
> > a copy (with strndup or etc), but then proto_tree_add_string will just take
> > its own copy. So I guess proto_tree_add_string_count is just an optimization
> > to avoid the extra copy.
>
> Sure, and user would not need to NUL terminate it if given field is worthy
> fetching (like: tree =! NULL), still this is penatly for not using
> proto_tree_add_item.
>
> So do we really need proto_tree_add_string_count()? or can we convert most
> of the calls to proto_tree_add_item() and some with proto_tree_add_string +
> tvb_get_string_enc.
If the string can be fetched directly from the TVB (even with encoding) then it
should use _add_item of course, with the appropriate ENC_ flag for ASCII or
UTF8 or EBCDIC or whatever.
> > The HTTP dissector is already using tvb_get_ptr for
> > performance reasons, so adding an extra copy kind of defeats the purpose of
> > that.
>
> I'm not quite sure about what code we're exactly talking about, in most
> cases I think proto_tree_add_item() should be same speed compared to
> proto_tree_add_string(.., tvb_get_ptr())
The problem lines in HTTP dissector are _add_string_format() because it wants
to do certain formatting operations on the displayed string. Perhaps the
problem is that we have no way of adding a string from the TVB but with unusual
formatting? Maybe the real answer is a proto_tree_add_item_format()?
(In reply to comment #23)
> (In reply to comment #21)
> > Not that it would be a bad thing if tvb_get_ptr went away entirely...
>
> I have just one thing to say about that:
>
> https://www.wireshark.org/~gerald/tvb_get_ptr.png
>
> :-)
Hah! I think we need to link to that in README.dissector and the tvb_get_ptr
doxygen :)
You are receiving this mail because:
- You are watching all bug changes.
