Wireshark mailing list archives

Re: Change in wireshark[master]: USB: do not take into account implicit structure alignment i...


From: Pascal Quantin <pascal.quantin () gmail com>
Date: Fri, 16 Jan 2015 10:42:07 +0100

2015-01-16 3:40 GMT+01:00 Guy Harris <guy () alum mit edu>:


On Jan 15, 2015, at 1:54 PM, "Pascal Quantin (Code Review)" <
code-review-do-not-reply () wireshark org> wrote:

Pascal Quantin has uploaded a new change for review.

 https://code.wireshark.org/review/6560

Change subject: USB: do not take into account implicit structure
alignment in USB address size
......................................................................

USB: do not take into account implicit structure alignment in USB
address size

"Compound" addresses, constructed from multiple non-contiguous components,
are beginning to be a bit of a problem; Michael Mann's recent changes
exposed the underlying problem with them, namely that we're using C data
structures as the address data for them, and C data structures might have
padding between elements and the end, and

        1) we do memcmp()-based comparisons on address data in, for
example, hash tables

and

        2) setting individual structure members from the components of the
address doesn't set the padding, so you get valgrind warnings that can
reflect real problems (wherein two addresses that are the same don't
compare equal) - I checked in my changes because a regression test showed
some captures being dissected differently because one version of TShark
found conversation matches and another version didn't, for the same capture.

Using wmem_new0() on them didn't seem to squelch the valgrind complaints;
if we need to do more, perhaps that's a sign that we need a better way to
handle "compound" addresses than stuffing them into C data structures.  I
don't know what the better way would be - just packing them into what
amount to packed structures, for example?  (No, GCC's keywords for
declaring structures packet don't suffice, as not all compilers used to
compile Wireshark are GCC-compatible.)



One approach could be to specialize more the addresses_equal() function so
as to check, per address type, the meaningful fields instead of the full
structure (including implicit padding). Of course it will have a cost in
terms of processing, but would prevent any further "hidden" regression in
the future. And as the function size could grow quite significantly, it
might be better not to inline it anymore.

Pascal.
___________________________________________________________________________
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: