Wireshark · Wireshark-dev: Re: [Wireshark-dev] [Wireshark-users] bit operations are missingfrom lua bindings?

[Sam Roberts <vieuxtech@xxxxxxxxx>]

2009/6/6 Stig Bjørlykke <stig@xxxxxxxxxxxxx>:
> On Fri, Jun 5, 2009 at 10:37 PM, Sam Roberts<vieuxtech@xxxxxxxxx> wrote:
>> Add this for up to 64 bit support, for completion:
>> [...]
>
> I have this code locally, but as you noted the UInt64 type does not
> work correctly.  The __tostring is not called when trying to convert a
> value to string (which leads to an error "string expected, got
> userdata"), and adding a __gc does nothing.  I will have a look at
> this later, unless someone beats me to it.

That's strange, I'm pretty sure I pulled a 40 bit field out of a tvb
and printed it. I'll look monday.

Though re-looking at __tostring, I think it leaks memory and truncates
the number, seee comments below.

>> Btw, I don't understand why the UInt64 userdata encapsulates a pointer
>> to a UInt64 instead of the uint itself. It makes it a bit harder to
>> use (you need the g_malloc()).
>
> This is noted in wslua_tvb.c:

I understand why we need a userdata, I mean why does it encapsulate a
__pointer__?

Most of wireshark's userdata encapsulate a pointer to an allocated C
struct, so the userdata wraps a pointer, but guint64s don't need to be
allocated:

Index: wslua.h
===================================================================
--- wslua.h	(revision 28648)
+++ wslua.h	(working copy)
@@ -220,7 +220,7 @@
 typedef struct _wslua_treeitem* TreeItem;
 typedef address* Address;
 typedef gint64* Int64;
-typedef guint64* UInt64;
+typedef guint64 UInt64;
 typedef header_field_info** Field;
 typedef field_info* FieldInfo;
 typedef struct _wslua_tap* Listener;
Index: wslua_tvb.c
===================================================================
--- wslua_tvb.c	(revision 28648)
+++ wslua_tvb.c	(working copy)
@@ -973,7 +973,7 @@
 WSLUA_METAMETHOD UInt64__tostring(lua_State* L) {
 	/* Converts the UInt64 into a string */
     UInt64 num = checkUInt64(L,1);
-    lua_pushstring(L,g_strdup_printf("%ld",(unsigned long int)*(num)));
+    lua_pushstring(L,g_strdup_printf("%ld",(unsigned long int)(num)));
     return 1;
 }

By the way, the existing code seems like it leaks the return value of
g_strdup_printf(), truncates the num, and treats unsigned as signed.
I'm not sure what wireshark conventions for printing 64-bit portably
is, with a recent libc I'd do:

   ..printf("%ju", (uintmax_t)num)


Anyhow, redefining the 64-bit userdata this way means no need to
allocate memory using g_malloc(), no need for a __gc, no memory leaks,
and code gets much simpler:

Index: wslua_field.c
===================================================================
--- wslua_field.c	(revision 28648)
+++ wslua_field.c	(working copy)
@@ -87,9 +87,7 @@
 			return 1;
 		}
 		case FT_UINT64: {
-			UInt64 num = g_malloc(sizeof(UInt64));
-			*num = fvalue_get_integer64(&(fi->value));
-			pushUInt64(L,num);
+			pushUInt64(L,fvalue_get_integer64(&fi->value));
 			return 1;
 		}
... plus all the other places...

I don't have a working compile env now to test and provide a complete
patch, but do you see what I mean?

Sam