2008/7/15 Jeff Morriss <[EMAIL PROTECTED]>:

>
> 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
> [email protected]
> https://wireshark.org/mailman/listinfo/wireshark-dev
>


 An easy way to solve alignment issues like this is to add a variable of the
data type that is of a size equal to that of the platform's pointer size.
This variable should be added as a member of a union with the original data
(an array of characters in this case).

Example:

The original data layout is this:

   char someData[50];

Casting someData to a data type other than char is never safe from an
alignment perspective. However if the data layout instead would be like
this:

   union AUnion
   {
      char someData[50];
      guint64 *ensuresProperAlignmentOfSomeData;
   };

This will force the compiler to allocate someData aligned on an 8-byte
boundary (safe to cast to any pointer on up to 64-bit platforms). Replacing
guint64 with a data type that varies in size with the platform (so that it
will be 32 bits on a 32 bit platform for instance) would of course be the
better approach, however I cannot recall if there is such a data type in
glib (there probably is one).

/ Peter
_______________________________________________
Wireshark-dev mailing list
[email protected]
https://wireshark.org/mailman/listinfo/wireshark-dev

Reply via email to