Bug 94570

--- spice/macros.h.orig	Thu Mar 10 15:14:49 2016
+++ spice/macros.h	Wed Mar 16 14:18:03 2016
@@ -381,6 +381,17 @@
 #define SPICE_ENDIAN_BIG    1234
 #define SPICE_ENDIAN_PDP    2143
 
+/* Lets see if we can use a standard header first... */
+#if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) \
+    && defined(BIG_ENDIAN)
+#  include <machine/endian.h>
+#  if BYTE_ORDER == LITTLE_ENDIAN
+#    define SPICE_ENDIAN SPICE_ENDIAN_LITTLE
+#  elif BYTE_ORDER == BIG_ENDIAN
+#    define SPICE_ENDIAN SPICE_ENDIAN_BIG
+#  endif
+#endif
+

So, instead of relying on those defined macros to include that header, why don't you check for it in configure.ac with AC_CHECK_HEADERS macro?

You would end with something like:

#if HAVE_MACHINE_ENDIAN_H
   #include <machine/endian.h>
...
#endif

Actually the code looks weird. First you verify that the three macros are all defined that you include an header to define again these macros.
Also the "standard header" comment is not true, at least if you consider C as the standard.

(In reply to Eduardo Lima (Etrunko) from comment #1)
> --- spice/macros.h.orig	Thu Mar 10 15:14:49 2016
> +++ spice/macros.h	Wed Mar 16 14:18:03 2016
> @@ -381,6 +381,17 @@
>  #define SPICE_ENDIAN_BIG    1234
>  #define SPICE_ENDIAN_PDP    2143
>  
> +/* Lets see if we can use a standard header first... */
> +#if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) \
> +    && defined(BIG_ENDIAN)
> +#  include <machine/endian.h>
> +#  if BYTE_ORDER == LITTLE_ENDIAN
> +#    define SPICE_ENDIAN SPICE_ENDIAN_LITTLE
> +#  elif BYTE_ORDER == BIG_ENDIAN
> +#    define SPICE_ENDIAN SPICE_ENDIAN_BIG
> +#  endif
> +#endif
> +
> 
> So, instead of relying on those defined macros to include that header, why
> don't you check for it in configure.ac with AC_CHECK_HEADERS macro?
> 
> You would end with something like:
> 
> #if HAVE_MACHINE_ENDIAN_H
>    #include <machine/endian.h>
> ...
> #endif

That won't work out of the box though, as macros.h is a public header. So all projects that use macros.h would have to explicitly add the AC_CHECK_HEADERS check for machine/endian.h, no?

Taking your latter comment into account, how about something along these lines?

+#if defined(BYTE_ORDER)
+#  if BYTE_ORDER == LITTLE_ENDIAN
+#    define SPICE_ENDIAN SPICE_ENDIAN_LITTLE
+#  elif BYTE_ORDER == BIG_ENDIAN
+#    define SPICE_ENDIAN SPICE_ENDIAN_BIG
+#  endif
+#endif

This would guard the code in the sense that it won't be evaluated if endian.h isn't already included through another header.

> 
> Taking your latter comment into account, how about something along these
> lines?
> 
> +#if defined(BYTE_ORDER)
> +#  if BYTE_ORDER == LITTLE_ENDIAN
> +#    define SPICE_ENDIAN SPICE_ENDIAN_LITTLE
> +#  elif BYTE_ORDER == BIG_ENDIAN
> +#    define SPICE_ENDIAN SPICE_ENDIAN_BIG
> +#  endif
> +#endif
> 
> This would guard the code in the sense that it won't be evaluated if
> endian.h isn't already included through another header.

Does this work on *BSD ?

Works fine on OpenBSD at least.

I was mistaken, it does require endian.h to work properly. I'll rework this diff to add a configure check so the inclusion is guarded.

Created attachment 123301 [details] [review]
Use _BYTE_ORDER to determine byte order

Created attachment 123302 [details] [review]
Check for machine/endian.h

Patch for spice-gtk to check for machine/endian.h

Created attachment 123405 [details] [review]
Use _BYTE_ORDER to determine byte order

Updated to actually use the originally intended header, endian.h.

Created attachment 123406 [details] [review]
Check for endian.h in spice-gtk

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/spice/spice-protocol/issues/1.