Wireshark-dev: Re: [Wireshark-dev] cast increases required alignment of target type in packet-d
Date Index · Thread Index · Other Months · All Mailing Lists
Date Prev · Date Next · Thread Prev · Thread Next
From: Jeff Morriss <jeff.morriss.ws@xxxxxxxxx>
Date: Tue, 15 Jul 2008 15:24:07 -0400

Good point, thanks!
(In fact if I had done a little more research and looked at the GLIB bug that made them put the void* cast in there: 

http://bugzilla.gnome.org/show_bug.cgi?id=502927

I would have seen that they verified the warning was bogus.)

Luis EG Ontanon wrote:

I think the cast to void* is OK.

the GArray is created given sizeof(struct contained_t) so it is going
to mallocate a block of N*sizeof(struct contained_t) and the very
first struct contained_t will be aligned o the base of the block given
by malloc (a void*), taking into account that compilers are supposed
to size structs into valid pointer boundaries, my guess is that every
struct contained_t in the GArray will be propperly aligned.


On Tue, Jul 15, 2008 at 4:50 PM, Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> wrote:

Hi folks,

My Solaris/SPARC compiles (with gcc-3.4.6) die with an alignment warning
in packet-diameter.c:


packet-diameter.c: In function `dissect_diameter_avp':
packet-diameter.c:340: warning: cast increases required alignment of target type

(followed by several more such warnings)

The relevant code for the first warning is:


    109 typedef struct _diam_vnd_t {
    110         guint32 code;
    111         GArray* vs_avps;
    112         GArray* vs_cmds;
    113 } diam_vnd_t;

[...]

    126 #define VND_AVP_VS(v) ((value_string*)((v)->vs_avps->data))

[...]

    319         const diam_vnd_t* vendor;

[...]

    340         vendor_avp_vs = VND_AVP_VS(vendor);

GArray comes from GLIB's garray.h:


     34 typedef struct _GArray          GArray;

[...]

     38 struct _GArray
     39 {
     40   gchar *data;
     41   guint len;
     42 };


The Solaris buildbot is not getting this warning but I think the warning
is valid: an array of chars may not be aligned correctly to be accessed
as a 'value_string' (a guint32 followed by a pointer).  And it may cause
bus errors because GArrays make copies of the data passed in--so we have
no guarantee how it will be aligned.

Unfortunately GArrays come in only 3 flavors: with pointers to chars,
pointers to guint8s, and pointers to pointers.

Is there an easy way to solve this?

Adding an intermediate cast to void*:

#define VND_AVP_VS(v) ((value_string*)(void*)((v)->vs_avps->data))

solves the warning and it appears this is the approach used by the glib
team:

http://svn.gnome.org/viewvc/glib?view=revision&revision=6092

but I don't think it's correct.  But at the moment I also think the
example (storing gint values) used in the GArrays documentation:

http://library.gnome.org/devel/glib/unstable/glib-Arrays.html

is also not correct/safe on SPARCs.  Can someone please tell me I'm wrong?
_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@xxxxxxxxxxxxx
https://wireshark.org/mailman/listinfo/wireshark-dev