On Thu, Oct 10, 2002 at 02:33:55AM +0200, Joerg Mayer wrote:
> I've looked into the source rpm of the suse 8.1 ethereal-0.9.6
> package and found one patch. The only thing that patch does is
> cast around some conversions between pointers and integers.
> As I didn't like that solution (I don't know whether this even
> compiles on 32bit archs)
If you're referring to the changes of "(gpointer) XXX" to
"(gpointer)((long int) XXX)", those are perfectly valid on 32-bit
platforms; at least for the platforms on which Ethereal works, those are
all ILP32 platforms, so "long int" and "int" are the same type.
The only effect of of those casts should be to suppress compiler
warnings; it shouldn't affect the actual code, whether it's being
compiled for a 32-bit ILP32 platform or a 64-bit LP64 platform.
GLib has "GINT_TO_POINTER()", "GUINT_TO_POINTER()",
"GPOINTER_TO_INT()", and "GPOINTER_TO_UINT()" macros, which, at least on
my x86 machine, just cast their argument appropriately; I don't know
whether they add the extra cast on 64-bit platforms or not - they're
defined in "glibconfig.h", so they *could* be platform-dependent, and
*could* add the extra cast. If they do, using those macros could be
considered the more "correct" way to do that and suppress the warning.
I created a solution that is more to
> my liking. There's basically 3 places where that needed to be
> done:
>
> 1) gtk/ I like my solution,
Unfortunately, passing pointers to automatic variables to
"g_signal_connect()" won't work, as when the callback for the signal is
called, those pointers won't point to the same variable (as that
variable won't exist), so, whilst your solution gets rid of the casts,
I don't think it'll work correctly. Even making the variable static
won't necessarily help, as the variable might have (in some cases,
*will* have) a different value at the time the callback is called than
at the time it's registered.
This won't work, either:
> - protocol_node = g_hash_table_lookup(proto_array,
> - (gpointer)proto_registrar_get_parent(i));
> + parent = proto_registrar_get_parent(i);
> + protocol_node = g_hash_table_lookup(proto_array, &parent);
Those are inequivalent - the hash table uses the parent's protocol ID,
cast to a pointer, so the latter won't work. You'd have to change the
hash table to use what the pointer points to - see, for example,
"g_int_equal()" and "g_int_hash()".
So, ugly though it may be, I'd vote for the SuSE solution here, unless
the GLib macros referred to above include the cast, in which case using
those macros would be equivalent to the SuSE solution but arguably a bit
cleaner.
> 2) packet-dcerpc.c and packet-rpc: I don't like just adding up
> integers and pointers in order to calculate a key, but I'm
> not sure that my solution of just removing the pointer from
> the sum is all that good either
In "packet-dcerpc.c", the "conv" member of a "dcerpc_call_key" points to
a conversation, and in "packet-rpc.c", the "conversation" member of an
"rpc_call_info_key" points to a conversation.
Conversations are assigned 32-bit unique IDs, which are intended for use
as hash keys, so you can do
return key->conv->index + key->ctx_id + key->smb_fid;
and
return key->xid + key->conversation->index;
as, at least from my reading of the code, the "conv" and "conversation"
pointers can never be null.
That gets rid of the integer+pointer addition without making the hash
function ignore the conversation value.
