Wireshark-dev: Re: [Wireshark-dev] signedness of comparison functions in ftype-integer.c
From: "Martin Mathieson" <martin.r.mathieson@xxxxxxxxxxxxxx>
Date: Mon, 8 Jan 2007 11:49:39 +0000
On 1/4/07, Martin Mathieson <martin.r.mathieson@xxxxxxxxxxxxxx> wrote:
On 1/3/07, Guy Harris <guy@xxxxxxxxxxxx> wrote: > Martin Mathieson wrote: > > > 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 > > I vote for solution 1 (and have everything that adds a signed value to > the protocol tree set the signed value in the union). The attached patch tries to implement (1), and seems to work (in the limited testing I've done). However, it ended up touching more files than I expected (a couple of dissectors even!). There are still 2 or 3 places where signed and unsigned FT types are still handled together by treating the value as guint32, thus potentially losing -ve values of FT_INT32 fields, I've preserved the existing behaviour for now. I would appreciate if someone could review this patch. It does correct a real problem, although I expect signed 32 bit fields with negative values are pretty rare in most protocols (I'd guess they're more likely to appear as generated fields). I suppose 64-bit numerical fields theoretically have the same problem, but is presumably even less likely to cause real problems...? Best regards, Martin
Hi, I've done more testing, and fixed some problems, the up-to-date patch is attached. The downside I can see to this change is that code using fvalue_get_integer() or get_value_integer() (both now in uinteger and sinteger versions) has to be careful to select the right one - calling the wrong one would result in an assertion if it doesn't fit the FT type of the field. See e.g. gtk/rtp_analysis.c's process_node() - I've made it always use the unsigned accesser, and it appears to be safe because all of the fields it currently retrieves are unsigned values. I believe that this change should be applied. If no-one objects, should it be done before or after the next release (not sure how imminent that might be) ? Best regards, Martin
Index: tap-iostat.c
===================================================================
--- tap-iostat.c (revision 20342)
+++ tap-iostat.c (working copy)
@@ -120,7 +120,7 @@
gp=proto_get_finfo_ptr_array(edt->tree, it->hf_index);
if(gp){
for(i=0;i<gp->len;i++){
- it->counter+=fvalue_get_integer(&((field_info *)gp->pdata[i])->value);
+ it->counter+=fvalue_get_uinteger(&((field_info *)gp->pdata[i])->value);
}
}
break;
@@ -138,7 +138,7 @@
case FT_UINT16:
case FT_UINT24:
case FT_UINT32:
- val=fvalue_get_integer(&((field_info *)gp->pdata[i])->value);
+ val=fvalue_get_uinteger(&((field_info *)gp->pdata[i])->value);
if((it->frames==1)&&(i==0)){
it->counter=val;
} else if(val<it->counter){
@@ -149,7 +149,7 @@
case FT_INT16:
case FT_INT24:
case FT_INT32:
- val=fvalue_get_integer(&((field_info *)gp->pdata[i])->value);
+ val=fvalue_get_sinteger(&((field_info *)gp->pdata[i])->value);
if((it->frames==1)&&(i==0)){
it->counter=val;
} else if((gint32)val<(gint32)(it->counter)){
@@ -183,7 +183,7 @@
case FT_UINT16:
case FT_UINT24:
case FT_UINT32:
- val=fvalue_get_integer(&((field_info *)gp->pdata[i])->value);
+ val=fvalue_get_uinteger(&((field_info *)gp->pdata[i])->value);
if((it->frames==1)&&(i==0)){
it->counter=val;
} else if(val>it->counter){
@@ -194,7 +194,7 @@
case FT_INT16:
case FT_INT24:
case FT_INT32:
- val=fvalue_get_integer(&((field_info *)gp->pdata[i])->value);
+ val=fvalue_get_sinteger(&((field_info *)gp->pdata[i])->value);
if((it->frames==1)&&(i==0)){
it->counter=val;
} else if((gint32)val>(gint32)(it->counter)){
@@ -233,7 +233,7 @@
case FT_INT16:
case FT_INT24:
case FT_INT32:
- val=fvalue_get_integer(&((field_info *)gp->pdata[i])->value);
+ val=fvalue_get_uinteger(&((field_info *)gp->pdata[i])->value);
it->counter+=val;
break;
case FT_RELATIVE_TIME:
Index: print.c
===================================================================
--- print.c (revision 20342)
+++ print.c (working copy)
@@ -373,7 +373,7 @@
fputs("\" value=\"", pdata->fh);
if (fi->hfinfo->bitmask!=0) {
- fprintf(pdata->fh, "%X", fvalue_get_integer(&fi->value));
+ fprintf(pdata->fh, "%X", fvalue_get_uinteger(&fi->value));
fputs("\" unmaskedvalue=\"", pdata->fh);
write_pdml_field_hex_value(pdata, fi);
}
@@ -440,7 +440,7 @@
if (g_ptr_array_len(finfo_array) < 1) {
return;
}
- num = fvalue_get_integer(&((field_info*)finfo_array->pdata[0])->value);
+ num = fvalue_get_uinteger(&((field_info*)finfo_array->pdata[0])->value);
g_ptr_array_free(finfo_array, FALSE);
/* frame.pkt_len --> geninfo.len */
@@ -448,7 +448,7 @@
if (g_ptr_array_len(finfo_array) < 1) {
return;
}
- len = fvalue_get_integer(&((field_info*)finfo_array->pdata[0])->value);
+ len = fvalue_get_uinteger(&((field_info*)finfo_array->pdata[0])->value);
g_ptr_array_free(finfo_array, FALSE);
/* frame.cap_len --> geninfo.caplen */
@@ -456,7 +456,7 @@
if (g_ptr_array_len(finfo_array) < 1) {
return;
}
- caplen = fvalue_get_integer(&((field_info*)finfo_array->pdata[0])->value);
+ caplen = fvalue_get_uinteger(&((field_info*)finfo_array->pdata[0])->value);
g_ptr_array_free(finfo_array, FALSE);
/* frame.time --> geninfo.timestamp */
Index: gtk/rtp_analysis.c
===================================================================
--- gtk/rtp_analysis.c (revision 20342)
+++ gtk/rtp_analysis.c (working copy)
@@ -3604,7 +3604,7 @@
*p_result = ipv4_get_net_order_addr(ipv4);
}
else {
- *p_result = fvalue_get_integer(&finfo->value);
+ *p_result = fvalue_get_uinteger(&finfo->value);
}
return TRUE;
}
Index: gtk/io_stat.c
===================================================================
--- gtk/io_stat.c (revision 20342)
+++ gtk/io_stat.c (working copy)
@@ -290,12 +290,21 @@
case FT_UINT16:
case FT_UINT24:
case FT_UINT32:
+ new_int=fvalue_get_uinteger(&((field_info *)gp->pdata[i])->value);
+
+ if((new_int>it->int_max)||(it->frames==0)){
+ it->int_max=new_int;
+ }
+ if((new_int<it->int_min)||(it->frames==0)){
+ it->int_min=new_int;
+ }
+ it->int_tot+=new_int;
+ break;
case FT_INT8:
case FT_INT16:
case FT_INT24:
case FT_INT32:
- new_int=fvalue_get_integer(&((field_info *)gp->pdata[i])->value);
-
+ new_int=fvalue_get_sinteger(&((field_info *)gp->pdata[i])->value);
if((new_int>it->int_max)||(it->frames==0)){
it->int_max=new_int;
}
Index: gtk/proto_draw.c
===================================================================
--- gtk/proto_draw.c (revision 20342)
+++ gtk/proto_draw.c (working copy)
@@ -1882,7 +1882,7 @@
gchar *url;
if(fi->hfinfo->type == FT_FRAMENUM) {
- cf_goto_frame(&cfile, fi->value.value.integer);
+ cf_goto_frame(&cfile, fi->value.value.uinteger);
}
if(FI_GET_FLAG(fi, FI_URL) && IS_FT_STRING(fi->hfinfo->type)) {
url = g_strndup(tvb_get_ptr(fi->ds_tvb, fi->start, fi->length), fi->length);
Index: epan/dfilter/semcheck.c
===================================================================
--- epan/dfilter/semcheck.c (revision 20342)
+++ epan/dfilter/semcheck.c (working copy)
@@ -133,7 +133,7 @@
fvalue_t *fv;
fv = fvalue_new(FT_UINT32);
- fvalue_set_integer(fv, val);
+ fvalue_set_uinteger(fv, val);
return fv;
}
Index: epan/ftypes/ftype-double.c
===================================================================
--- epan/ftypes/ftype-double.c (revision 20342)
+++ epan/ftypes/ftype-double.c (working copy)
@@ -169,12 +169,14 @@
float_val_repr_len, /* len_string_repr */
NULL, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_snteger */
NULL, /* set_value_integer64 */
double_fvalue_set_floating, /* set_value_floating */
NULL, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
value_get_floating, /* get_value_floating */
@@ -205,12 +207,14 @@
double_val_repr_len, /* len_string_repr */
NULL, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
double_fvalue_set_floating, /* set_value_floating */
NULL, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_snteger */
NULL, /* get_value_integer64 */
value_get_floating, /* get_value_floating */
Index: epan/ftypes/ftype-ipv4.c
===================================================================
--- epan/ftypes/ftype-ipv4.c (revision 20342)
+++ epan/ftypes/ftype-ipv4.c (working copy)
@@ -33,7 +33,7 @@
static void
-set_integer(fvalue_t *fv, guint32 value)
+set_uinteger(fvalue_t *fv, guint32 value)
{
ipv4_addr_set_net_order_addr(&(fv->value.ipv4), value);
ipv4_addr_set_netmask_bits(&(fv->value.ipv4), 32);
@@ -99,7 +99,7 @@
if (!nmask_fvalue) {
return FALSE;
}
- nmask_bits = fvalue_get_integer(nmask_fvalue);
+ nmask_bits = fvalue_get_uinteger(nmask_fvalue);
FVALUE_FREE(nmask_fvalue);
if (nmask_bits > 32) {
@@ -206,12 +206,14 @@
val_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftypes.c
===================================================================
--- epan/ftypes/ftypes.c (revision 20342)
+++ epan/ftypes/ftypes.c (working copy)
@@ -417,13 +417,21 @@
}
void
-fvalue_set_integer(fvalue_t *fv, guint32 value)
+fvalue_set_uinteger(fvalue_t *fv, guint32 value)
{
- g_assert(fv->ftype->set_value_integer);
- fv->ftype->set_value_integer(fv, value);
+ g_assert(fv->ftype->set_value_uinteger);
+ fv->ftype->set_value_uinteger(fv, value);
}
void
+fvalue_set_sinteger(fvalue_t *fv, gint32 value)
+{
+ g_assert(fv->ftype->set_value_sinteger);
+ fv->ftype->set_value_sinteger(fv, value);
+}
+
+
+void
fvalue_set_integer64(fvalue_t *fv, guint64 value)
{
g_assert(fv->ftype->set_value_integer64);
@@ -446,12 +454,20 @@
}
guint32
-fvalue_get_integer(fvalue_t *fv)
+fvalue_get_uinteger(fvalue_t *fv)
{
- g_assert(fv->ftype->get_value_integer);
- return fv->ftype->get_value_integer(fv);
+ g_assert(fv->ftype->get_value_uinteger);
+ return fv->ftype->get_value_uinteger(fv);
}
+gint32
+fvalue_get_sinteger(fvalue_t *fv)
+{
+ g_assert(fv->ftype->get_value_sinteger);
+ return fv->ftype->get_value_sinteger(fv);
+}
+
+
guint64
fvalue_get_integer64(fvalue_t *fv)
{
Index: epan/ftypes/ftype-bytes.c
===================================================================
--- epan/ftypes/ftype-bytes.c (revision 20342)
+++ epan/ftypes/ftype-bytes.c (working copy)
@@ -500,12 +500,14 @@
bytes_repr_len, /* len_string_repr */
bytes_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -536,12 +538,14 @@
bytes_repr_len, /* len_string_repr */
bytes_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -572,12 +576,14 @@
bytes_repr_len, /* len_string_repr */
ether_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -608,12 +614,14 @@
ipv6_repr_len, /* len_string_repr */
ipv6_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -644,12 +652,14 @@
oid_repr_len, /* len_string_repr */
oid_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftype-guid.c
===================================================================
--- epan/ftypes/ftype-guid.c (revision 20342)
+++ epan/ftypes/ftype-guid.c (working copy)
@@ -141,12 +141,14 @@
guid_repr_len, /* len_string_repr */
guid_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftype-pcre.c
===================================================================
--- epan/ftypes/ftype-pcre.c (revision 20342)
+++ epan/ftypes/ftype-pcre.c (working copy)
@@ -232,12 +232,14 @@
NULL, /* len_string_repr */
NULL, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftypes.h
===================================================================
--- epan/ftypes/ftypes.h (revision 20342)
+++ epan/ftypes/ftypes.h (working copy)
@@ -149,7 +149,8 @@
union {
/* Put a few basic types in here */
gpointer pointer;
- guint32 integer;
+ guint32 uinteger;
+ gint32 sinteger;
guint64 integer64;
gdouble floating;
gchar *string;
@@ -181,12 +182,14 @@
typedef int (*FvalueStringReprLen)(fvalue_t*, ftrepr_t);
typedef void (*FvalueSetFunc)(fvalue_t*, gpointer, gboolean);
-typedef void (*FvalueSetIntegerFunc)(fvalue_t*, guint32);
+typedef void (*FvalueSetUnsignedIntegerFunc)(fvalue_t*, guint32);
+typedef void (*FvalueSetSignedIntegerFunc)(fvalue_t*, gint32);
typedef void (*FvalueSetInteger64Func)(fvalue_t*, guint64);
typedef void (*FvalueSetFloatingFunc)(fvalue_t*, gdouble);
typedef gpointer (*FvalueGetFunc)(fvalue_t*);
-typedef guint32 (*FvalueGetIntegerFunc)(fvalue_t*);
+typedef guint32 (*FvalueGetUnsignedIntegerFunc)(fvalue_t*);
+typedef gint32 (*FvalueGetSignedIntegerFunc)(fvalue_t*);
typedef guint64 (*FvalueGetInteger64Func)(fvalue_t*);
typedef double (*FvalueGetFloatingFunc)(fvalue_t*);
@@ -209,13 +212,15 @@
/* could be union */
FvalueSetFunc set_value;
- FvalueSetIntegerFunc set_value_integer;
+ FvalueSetUnsignedIntegerFunc set_value_uinteger;
+ FvalueSetSignedIntegerFunc set_value_sinteger;
FvalueSetInteger64Func set_value_integer64;
FvalueSetFloatingFunc set_value_floating;
/* could be union */
FvalueGetFunc get_value;
- FvalueGetIntegerFunc get_value_integer;
+ FvalueGetUnsignedIntegerFunc get_value_uinteger;
+ FvalueGetSignedIntegerFunc get_value_sinteger;
FvalueGetInteger64Func get_value_integer64;
FvalueGetFloatingFunc get_value_floating;
@@ -299,9 +304,12 @@
fvalue_set(fvalue_t *fv, gpointer value, gboolean already_copied);
void
-fvalue_set_integer(fvalue_t *fv, guint32 value);
+fvalue_set_uinteger(fvalue_t *fv, guint32 value);
void
+fvalue_set_sinteger(fvalue_t *fv, gint32 value);
+
+void
fvalue_set_integer64(fvalue_t *fv, guint64 value);
void
@@ -311,8 +319,11 @@
fvalue_get(fvalue_t *fv);
extern guint32
-fvalue_get_integer(fvalue_t *fv);
+fvalue_get_uinteger(fvalue_t *fv);
+extern gint32
+fvalue_get_sinteger(fvalue_t *fv);
+
guint64
fvalue_get_integer64(fvalue_t *fv);
Index: epan/ftypes/ftype-tvbuff.c
===================================================================
--- epan/ftypes/ftype-tvbuff.c (revision 20342)
+++ epan/ftypes/ftype-tvbuff.c (working copy)
@@ -272,12 +272,14 @@
val_repr_len, /* len_string_repr */
value_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftype-integer.c
===================================================================
--- epan/ftypes/ftype-integer.c (revision 20342)
+++ epan/ftypes/ftype-integer.c (working copy)
@@ -48,21 +48,35 @@
static void
int_fvalue_new(fvalue_t *fv)
{
- fv->value.integer = 0;
+ fv->value.uinteger = 0;
}
static void
-set_integer(fvalue_t *fv, guint32 value)
+set_uinteger(fvalue_t *fv, guint32 value)
{
- fv->value.integer = value;
+ fv->value.uinteger = value;
}
+static void
+set_sinteger(fvalue_t *fv, gint32 value)
+{
+ fv->value.sinteger = value;
+}
+
+
static guint32
-get_integer(fvalue_t *fv)
+get_uinteger(fvalue_t *fv)
{
- return fv->value.integer;
+ return fv->value.uinteger;
}
+static gint32
+get_sinteger(fvalue_t *fv)
+{
+ return fv->value.sinteger;
+}
+
+
static gboolean
val_from_unparsed(fvalue_t *fv, char *s, gboolean allow_partial_value _U_, LogFunc logfunc)
{
@@ -104,7 +118,7 @@
return FALSE;
}
- fv->value.integer = value;
+ fv->value.uinteger = value;
return TRUE;
}
@@ -117,7 +131,7 @@
static void
integer_to_repr(fvalue_t *fv, ftrepr_t rtype _U_, char *buf)
{
- sprintf(buf, "%d", fv->value.integer);
+ sprintf(buf, "%d", fv->value.sinteger);
}
static int
@@ -129,7 +143,7 @@
static void
uinteger_to_repr(fvalue_t *fv, ftrepr_t rtype _U_, char *buf)
{
- sprintf(buf, "%u", fv->value.integer);
+ sprintf(buf, "%u", fv->value.uinteger);
}
static gboolean
@@ -149,7 +163,7 @@
val = get_ipxnet_addr(s, &known);
if (known) {
- fv->value.integer = val;
+ fv->value.uinteger = val;
return TRUE;
}
@@ -166,73 +180,73 @@
static void
ipxnet_to_repr(fvalue_t *fv, ftrepr_t rtype _U_, char *buf)
{
- sprintf(buf, "0x%08x", fv->value.integer);
+ sprintf(buf, "0x%08x", fv->value.uinteger);
}
static gboolean
cmp_eq(fvalue_t *a, fvalue_t *b)
{
- return a->value.integer == b->value.integer;
+ return a->value.uinteger == b->value.uinteger;
}
static gboolean
cmp_ne(fvalue_t *a, fvalue_t *b)
{
- return a->value.integer != b->value.integer;
+ return a->value.uinteger != b->value.uinteger;
}
static gboolean
u_cmp_gt(fvalue_t *a, fvalue_t *b)
{
- return (int)a->value.integer > (int)b->value.integer;
+ return a->value.uinteger > b->value.uinteger;
}
static gboolean
u_cmp_ge(fvalue_t *a, fvalue_t *b)
{
- return (int)a->value.integer >= (int)b->value.integer;
+ return a->value.uinteger >= b->value.uinteger;
}
static gboolean
u_cmp_lt(fvalue_t *a, fvalue_t *b)
{
- return (int)a->value.integer < (int)b->value.integer;
+ return a->value.uinteger < b->value.uinteger;
}
static gboolean
u_cmp_le(fvalue_t *a, fvalue_t *b)
{
- return (int)a->value.integer <= (int)b->value.integer;
+ return a->value.uinteger <= b->value.uinteger;
}
static gboolean
s_cmp_gt(fvalue_t *a, fvalue_t *b)
{
- return a->value.integer > b->value.integer;
+ return a->value.sinteger > b->value.sinteger;
}
static gboolean
s_cmp_ge(fvalue_t *a, fvalue_t *b)
{
- return a->value.integer >= b->value.integer;
+ return a->value.sinteger >= b->value.sinteger;
}
static gboolean
s_cmp_lt(fvalue_t *a, fvalue_t *b)
{
- return a->value.integer < b->value.integer;
+ return a->value.sinteger < b->value.sinteger;
}
static gboolean
s_cmp_le(fvalue_t *a, fvalue_t *b)
{
- return a->value.integer <= b->value.integer;
+ return a->value.sinteger <= b->value.sinteger;
}
static gboolean
cmp_bitwise_and(fvalue_t *a, fvalue_t *b)
{
- return ((a->value.integer & b->value.integer) != 0);
+ return ((a->value.uinteger & b->value.uinteger) != 0);
}
static void
@@ -393,7 +407,7 @@
static void
boolean_fvalue_new(fvalue_t *fv)
{
- fv->value.integer = TRUE;
+ fv->value.uinteger = TRUE;
}
static int
@@ -405,15 +419,15 @@
static void
boolean_to_repr(fvalue_t *fv, ftrepr_t rtype _U_, char *buf)
{
- sprintf(buf, "%s", fv->value.integer ? "1" : "0");
+ sprintf(buf, "%s", fv->value.uinteger ? "1" : "0");
}
/* Checks for equality with zero or non-zero */
static gboolean
bool_eq(fvalue_t *a, fvalue_t *b)
{
- if (a->value.integer) {
- if (b->value.integer) {
+ if (a->value.uinteger) {
+ if (b->value.uinteger) {
return TRUE;
}
else {
@@ -421,7 +435,7 @@
}
}
else {
- if (b->value.integer) {
+ if (b->value.uinteger) {
return FALSE;
}
else {
@@ -456,12 +470,14 @@
uinteger_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ get_uinteger, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -491,12 +507,14 @@
uinteger_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ get_uinteger, /* get_value_integer */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -526,12 +544,14 @@
uinteger_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_integer */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ get_uinteger, /* get_value_integer */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -561,12 +581,14 @@
uinteger_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ get_uinteger, /* get_value_integer */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -596,13 +618,15 @@
uinteger64_repr_len, /* len_string_repr */
NULL, /* set_value */
- NULL, /* set_value_integer */
- set_integer64, /* set_value_integer64 */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
+ set_integer64, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- NULL, /* get_value_integer */
- get_integer64, /* get_value_integer64 */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
+ get_integer64, /* get_value_integer64 */
NULL, /* get_value_floating */
cmp_eq64,
@@ -631,12 +655,14 @@
integer_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ set_sinteger, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ get_sinteger, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -666,12 +692,14 @@
integer_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ set_sinteger, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ get_sinteger, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -701,12 +729,14 @@
integer_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ set_sinteger, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ get_sinteger, /* get_value_integer */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -736,12 +766,14 @@
integer_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ set_sinteger, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ get_sinteger, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -771,13 +803,15 @@
integer64_repr_len, /* len_string_repr */
NULL, /* set_value */
- NULL, /* set_value_integer */
- set_integer64, /* set_value_integer64 */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
+ set_integer64, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- NULL, /* get_value_integer */
- get_integer64, /* get_value_integer64 */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
+ get_integer64, /* get_value_integer64 */
NULL, /* get_value_floating */
cmp_eq64,
@@ -806,12 +840,14 @@
boolean_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ get_uinteger, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -842,12 +878,14 @@
ipxnet_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ get_uinteger, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -878,12 +916,14 @@
uinteger_repr_len, /* len_string_repr */
NULL, /* set_value */
- set_integer, /* set_value_integer */
+ set_uinteger, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- get_integer, /* get_value_integer */
+ get_uinteger, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftype-time.c
===================================================================
--- epan/ftypes/ftype-time.c (revision 20342)
+++ epan/ftypes/ftype-time.c (working copy)
@@ -357,12 +357,14 @@
absolute_val_repr_len, /* len_string_repr */
time_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -392,12 +394,14 @@
relative_val_repr_len, /* len_string_repr */
time_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftype-none.c
===================================================================
--- epan/ftypes/ftype-none.c (revision 20342)
+++ epan/ftypes/ftype-none.c (working copy)
@@ -44,12 +44,14 @@
NULL, /* len_string_repr */
NULL, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
NULL, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/ftypes/ftype-string.c
===================================================================
--- epan/ftypes/ftype-string.c (revision 20342)
+++ epan/ftypes/ftype-string.c (working copy)
@@ -309,12 +309,14 @@
string_repr_len, /* len_string_repr */
string_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -344,12 +346,14 @@
string_repr_len, /* len_string_repr */
string_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
@@ -379,12 +383,14 @@
string_repr_len, /* len_string_repr */
string_fvalue_set, /* set_value */
- NULL, /* set_value_integer */
+ NULL, /* set_value_uinteger */
+ NULL, /* set_value_sinteger */
NULL, /* set_value_integer64 */
NULL, /* set_value_floating */
value_get, /* get_value */
- NULL, /* get_value_integer */
+ NULL, /* get_value_uinteger */
+ NULL, /* get_value_sinteger */
NULL, /* get_value_integer64 */
NULL, /* get_value_floating */
Index: epan/wslua/wslua_field.c
===================================================================
--- epan/wslua/wslua_field.c (revision 20342)
+++ epan/wslua/wslua_field.c (working copy)
@@ -68,11 +68,13 @@
case FT_UINT24:
case FT_UINT32:
case FT_FRAMENUM:
+ lua_pushnumber(L,(lua_Number)fvalue_get_uinteger(&(fi->value)));
+ return 1;
case FT_INT8:
case FT_INT16:
case FT_INT24:
case FT_INT32:
- lua_pushnumber(L,(lua_Number)fvalue_get_integer(&(fi->value)));
+ lua_pushnumber(L,(lua_Number)fvalue_get_sinteger(&(fi->value)));
return 1;
case FT_FLOAT:
case FT_DOUBLE:
Index: epan/proto.c
===================================================================
--- epan/proto.c (revision 20342)
+++ epan/proto.c (working copy)
@@ -1450,7 +1450,7 @@
static void
proto_tree_set_ipxnet(field_info *fi, guint32 value)
{
- fvalue_set_integer(&fi->value, value);
+ fvalue_set_uinteger(&fi->value, value);
}
/* Add a FT_IPv4 to a proto_tree */
@@ -1531,7 +1531,7 @@
static void
proto_tree_set_ipv4(field_info *fi, guint32 value)
{
- fvalue_set_integer(&fi->value, value);
+ fvalue_set_uinteger(&fi->value, value);
}
/* Add a FT_IPv6 to a proto_tree */
@@ -2403,7 +2403,7 @@
integer >>= hfinfo->bitshift;
}
}
- fvalue_set_integer(&fi->value, integer);
+ fvalue_set_uinteger(&fi->value, integer);
}
/* Add FT_UINT64 to a proto_tree */
@@ -2567,7 +2567,7 @@
integer >>= hfinfo->bitshift;
}
}
- fvalue_set_integer(&fi->value, integer);
+ fvalue_set_sinteger(&fi->value, integer);
}
/* Add FT_INT64 to a proto_tree */
@@ -3802,7 +3802,7 @@
break;
case FT_IPXNET:
- integer = fvalue_get_integer(&fi->value);
+ integer = fvalue_get_uinteger(&fi->value);
ret = g_snprintf(label_str, ITEM_LABEL_LENGTH,
"%s: %s (0x%08X)", hfinfo->name,
get_ipxnet_name(integer), integer);
@@ -3909,7 +3909,7 @@
tfstring = (const struct true_false_string*) hfinfo->strings;
}
- value = fvalue_get_integer(&fi->value);
+ value = fvalue_get_uinteger(&fi->value);
if (hfinfo->bitmask) {
/* Figure out the bit width */
bitwidth = hfinfo_bitwidth(hfinfo);
@@ -3954,7 +3954,7 @@
format = hfinfo_uint_vals_format(hfinfo);
/* Un-shift bits */
- unshifted_value = fvalue_get_integer(&fi->value);
+ unshifted_value = fvalue_get_uinteger(&fi->value);
value = unshifted_value;
if (hfinfo->bitshift > 0) {
unshifted_value <<= hfinfo->bitshift;
@@ -3991,7 +3991,7 @@
format = hfinfo_uint_format(hfinfo);
/* Un-shift bits */
- unshifted_value = fvalue_get_integer(&fi->value);
+ unshifted_value = fvalue_get_uinteger(&fi->value);
value = unshifted_value;
if (hfinfo->bitshift > 0) {
unshifted_value <<= hfinfo->bitshift;
@@ -4025,7 +4025,7 @@
/* Pick the proper format string */
format = hfinfo_uint_vals_format(hfinfo);
- value = fvalue_get_integer(&fi->value);
+ value = fvalue_get_uinteger(&fi->value);
/* Fill in the textual info */
ret = g_snprintf(label_str, ITEM_LABEL_LENGTH,
@@ -4045,7 +4045,7 @@
/* Pick the proper format string */
format = hfinfo_uint_format(hfinfo);
- value = fvalue_get_integer(&fi->value);
+ value = fvalue_get_uinteger(&fi->value);
/* Fill in the textual info */
if (IS_BASE_DUAL(hfinfo->display)) {
@@ -4093,7 +4093,7 @@
/* Pick the proper format string */
format = hfinfo_int_vals_format(hfinfo);
- value = fvalue_get_integer(&fi->value);
+ value = fvalue_get_sinteger(&fi->value);
/* Fill in the textual info */
ret = g_snprintf(label_str, ITEM_LABEL_LENGTH,
@@ -4113,7 +4113,7 @@
/* Pick the proper format string */
format = hfinfo_int_format(hfinfo);
- value = fvalue_get_integer(&fi->value);
+ value = fvalue_get_sinteger(&fi->value);
/* Fill in the textual info */
if (IS_BASE_DUAL(hfinfo->display)) {
@@ -5076,6 +5076,7 @@
int dfilter_len, i;
gint start, length, length_remaining;
guint8 c;
+ gchar is_signed_num = FALSE;
hfinfo = finfo->hfinfo;
DISSECTOR_ASSERT(hfinfo);
@@ -5103,14 +5104,15 @@
*/
switch(hfinfo->type) {
+ case FT_INT8:
+ case FT_INT16:
+ case FT_INT24:
+ case FT_INT32:
+ is_signed_num = TRUE;
case FT_UINT8:
case FT_UINT16:
case FT_UINT24:
case FT_UINT32:
- case FT_INT8:
- case FT_INT16:
- case FT_INT24:
- case FT_INT32:
case FT_FRAMENUM:
/*
* 4 bytes for " == ".
@@ -5134,7 +5136,8 @@
format = hfinfo_numeric_format(hfinfo);
g_snprintf(*filter, dfilter_len, format,
hfinfo->abbrev,
- fvalue_get_integer(&finfo->value));
+ is_signed_num ? fvalue_get_sinteger(&finfo->value)
+ : fvalue_get_uinteger(&finfo->value));
}
break;
Index: epan/dissectors/packet-smb-sidsnooping.c
===================================================================
--- epan/dissectors/packet-smb-sidsnooping.c (revision 20342)
+++ epan/dissectors/packet-smb-sidsnooping.c (working copy)
@@ -124,7 +124,7 @@
return 0;
}
fi=gp->pdata[0];
- info_level=fi->value.value.integer;
+ info_level=fi->value.value.sinteger;
if(info_level!=1){
return 0;
@@ -209,7 +209,7 @@
fi_name=gp_names->pdata[num_rids-1];
strncpy(sid_name, sid, len);
sid_name[len++]='-';
- g_snprintf(sid_name+len, 256-len, "%d",fi_rid->value.value.integer);
+ g_snprintf(sid_name+len, 256-len, "%d",fi_rid->value.value.sinteger);
add_sid_name_mapping(sid_name, fi_name->value.value.string);
}
return 1;
@@ -235,7 +235,7 @@
return 0;
}
fi=gp->pdata[0];
- info_level=fi->value.value.integer;
+ info_level=fi->value.value.sinteger;
switch(info_level){
case 3:
Index: epan/dissectors/packet-ncp2222.inc
===================================================================
--- epan/dissectors/packet-ncp2222.inc (revision 20342)
+++ epan/dissectors/packet-ncp2222.inc (working copy)
@@ -1663,10 +1663,10 @@
since we sometimes fake the entries to speed things up.
this dissector should not call fvalue_ functions directly.
*/
- if(!finfo->value.ftype->get_value_integer){
+ if(!finfo->value.ftype->get_value_uinteger){
return 0;
}
- return fvalue_get_integer(&finfo->value);
+ return fvalue_get_uinteger(&finfo->value);
}
static guint
get_item_value(proto_item *item)
Index: file.c
===================================================================
--- file.c (revision 20342)
+++ file.c (working copy)
@@ -3088,7 +3088,7 @@
hfinfo = cf->finfo_selected->hfinfo;
g_assert(hfinfo);
if (hfinfo->type == FT_FRAMENUM) {
- framenum = fvalue_get_integer(&cf->finfo_selected->value);
+ framenum = fvalue_get_uinteger(&cf->finfo_selected->value);
if (framenum != 0)
return cf_goto_frame(cf, framenum);
}
- Follow-Ups:
- Re: [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- From: Martin Mathieson
- Re: [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- References:
- [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- From: Martin Mathieson
- Re: [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- From: Guy Harris
- Re: [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- From: Martin Mathieson
- [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- Prev by Date: Re: [Wireshark-dev] Microsoft Visual C Version 6 support isa bitoutdated ...
- Next by Date: Re: [Wireshark-dev] Microsoft Visual C Version 6 support isa bitoutdated ...
- Previous by thread: Re: [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- Next by thread: Re: [Wireshark-dev] signedness of comparison functions in ftype-integer.c
- Index(es):