Re: Is this the bug of wmem_tree_lookup32_array_le()?

[John Thacker <johnthacker () gmail com>]

On Sun, Oct 17, 2021 at 5:41 AM qiangxiong.huang via Wireshark-dev <
wireshark-dev () wireshark org> wrote:

> Who knows that the current behavior of the wmem_tree_lookup32_array_le()
> is designed in this way or it is just a bug?
> If I want to fix the issue #17633, should I modify this
> wmem_tree_lookup32_array_le() directly or write a new function like
> wmem_tree_lookup32_array_le_key_as_big_num() (or other name you recommend)?

While I fully understand why you would want a lookup that would be a
lexicographical sort with the order of the 32 bit integers in the key being
rank (in order to handle a 64 bit integer as two 32 bit integers), the
current dissectors that use the API depend on the current behavior and
would break, e.g.
https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-dns.c#L4184-L4185
and
https://gitlab.com/wireshark/wireshark/-/blob/master/epan/dissectors/packet-usb.c#L3560-L3574

Generally these dissectors have several independent keys, and what they
want to match is an exact match on some keys (transaction ID, for example)
and a less than match on others (e.g., frame number.) They do the less than
or equal match and then check for strict equality on the keys that need
that. Handling the lookup like a lexicographical sort would give incorrect
results for those dissectors; the current behavior is as intended.

So if you're going to do it, you need a new API function. But perhaps what
you really want is to implement something like wmem_tree_lookup64 that
supports 64 bit integers directly rather than expecting the callers to
separate into the upper and lower 32 bits. It is possible to do lookups of
other types (there's a string lookup using a string comparison function,
which might be worth looking at since the method for 32 bit integers calls
the GUINT_TO_POINTER macros and is guaranteed to work for 64 bit integers.)

John Thacker

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