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 structurealignment in USB address size...................................................................... USB: do not take into account implicit structure alignment in USBaddress 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:
- Re: Change in wireshark[master]: USB: do not take into account implicit structure alignment i... Guy Harris (Jan 15)
- Re: Change in wireshark[master]: USB: do not take into account implicit structure alignment i... Pascal Quantin (Jan 16)