Wireshark-dev: [Wireshark-dev] signedness of comparison functions in ftype-integer.c

From: "Martin Mathieson" <martin.r.mathieson@xxxxxxxxxxxxxx>
Date: Wed, 3 Jan 2007 16:45:49 +0000

Hi,

While looking at bug 1279 (Negative values for RTCP round trip delay
cannot be stored in guint32) I noticed that I couldn't use filter
expressions using < and negative numbers with an FT_INT32 field.  The
problem seems to be that:
- fvalue_t.value.integer is a guint32
- the current less-than function for FT_INT32 (s_cmp_lt) doesn't cast
these values back to integers, and sees small -ve numbers as large
+ves.

In fact, it looks as if the signed functions don't, but should cast
the values to their actual signed types, wheres the unsigned functions
shouldn't, but do, cast the values to the wrong types, and therefore
potentially lose the msb from their value!!!

The following patch gets me past the immediate problem I saw.

===================================================================
--- ftype-integer.c     (revision 20293)
+++ ftype-integer.c     (working copy)
@@ -220,7 +220,7 @@
static gboolean
s_cmp_lt(fvalue_t *a, fvalue_t *b)
{
-       return a->value.integer < b->value.integer;
+       return (int)a->value.integer < (int)b->value.integer;
}

For the more general problem, I see 2 possible solutions:
(1) have both signed and values in the union, and use the appropriate
signed or unsigned parts of the union in the comparison functions
(2) leave the union as it is with unsigned members, cast values in all
the signed versions instead of the unsigned versions

Or am I missing something very obvious?

Best regards,
Martin