2009/6/6 Stig Bjørlykke <s...@bjorlykke.org>:
> On Fri, Jun 5, 2009 at 10:37 PM, Sam Roberts<vieuxt...@gmail.com> 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
___________________________________________________________________________
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-requ...@wireshark.org?subject=unsubscribe

Reply via email to