Re: [PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Alexandre Julliard]

Hilko Bengen <bengen(a)hilluzination.de> writes:
> @@ -46,6 +46,8 @@
>
> #include "pshpack1.h"
>
> +#include <endian.h>

endian.h is not portable, you should use the endianness defines from the
Wine headers instead.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Hilko Bengen]

* Alexandre Julliard:
> endian.h is not portable, you should use the endianness defines from
> the Wine headers instead.

So, do I need to define my own htobe16 and htobe32 functions or macros
based on whether WORDS_BIGENDIAN is defined or not?

-Hilko

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Alexandre Julliard]

Hilko Bengen <bengen(a)hilluzination.de> writes:
> * Alexandre Julliard:
> > endian.h is not portable, you should use the endianness defines from
> > the Wine headers instead.
>
> So, do I need to define my own htobe16 and htobe32 functions or macros
> based on whether WORDS_BIGENDIAN is defined or not?

Yes.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Hilko Bengen]

* Alexandre Julliard:
> Hilko Bengen <bengen(a)hilluzination.de> writes:
> > So, do I need to define my own htobe16 and htobe32 functions or macros
> > based on whether WORDS_BIGENDIAN is defined or not?
>
> Yes.

I have tested these macros on Linux/PowerPC and they still produce the
same results as on i386.

Is this form okay?

-Hilko

diff --git a/tools/sfnt2fnt.c b/tools/sfnt2fnt.c
index df9260f..9ce503a 100644
--- a/tools/sfnt2fnt.c
+++ b/tools/sfnt2fnt.c
@@ -46,7 +46,20 @@
 
 #include "pshpack1.h"
 
-#include <endian.h>
+#if !defined(htole16) || !defined(htole32)
+#include "basetsd.h"
+#if defined(WORDS_BIGENDIAN)
+#define htole16(_x) ( (((_x) & 0xff) << 8) |            \
+                      (((_x) & 0xff00) >> 8) )
+#define htole32(_x) ( (((_x) & 0xff) << 24) |           \
+                      (((_x) & 0xff00) << 8) |          \
+                      (((_x) & 0xff0000) >> 8) |        \
+                      (((_x) & 0xff000000) >> 24) )
+#else
+#define htole16(_x) (_x)
+#define htole32(_x) (_x)
+#endif
+#endif
 
 typedef struct
 {

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[André Hentschel]

Am 15.06.2012 19:52, schrieb Hilko Bengen:
> * Alexandre Julliard:
> > Hilko Bengen <bengen(a)hilluzination.de> writes:
> > > So, do I need to define my own htobe16 and htobe32 functions or macros
> > > based on whether WORDS_BIGENDIAN is defined or not?
> >
> > Yes.
>
> I have tested these macros on Linux/PowerPC and they still produce the
> same results as on i386.
>
> Is this form okay?
>
> -Hilko
>
> diff --git a/tools/sfnt2fnt.c b/tools/sfnt2fnt.c
> index df9260f..9ce503a 100644
> --- a/tools/sfnt2fnt.c
> +++ b/tools/sfnt2fnt.c
> @@ -46,7 +46,20 @@
>
> #include "pshpack1.h"
>
> -#include <endian.h>
> +#if !defined(htole16) || !defined(htole32)
> +#include "basetsd.h"
> +#if defined(WORDS_BIGENDIAN)
> +#define htole16(_x) ( (((_x) & 0xff) << 8) |            \
> +                      (((_x) & 0xff00) >> 8) )
> +#define htole32(_x) ( (((_x) & 0xff) << 24) |           \
> +                      (((_x) & 0xff00) << 8) |          \
> +                      (((_x) & 0xff0000) >> 8) |        \
> +                      (((_x) & 0xff000000) >> 24) )
> +#else
> +#define htole16(_x) (_x)
> +#define htole32(_x) (_x)
> +#endif
> +#endif
>
> typedef struct
>  {

-intendation would make this more readable (even for review)
-maybe one block per function








-- 

Best Regards, André Hentschel

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Alexandre Julliard]

Hilko Bengen <bengen(a)hilluzination.de> writes:
> * Alexandre Julliard:
> > Hilko Bengen <bengen(a)hilluzination.de> writes:
> > > So, do I need to define my own htobe16 and htobe32 functions or macros
> > > based on whether WORDS_BIGENDIAN is defined or not?
> >
> > Yes.
>
> I have tested these macros on Linux/PowerPC and they still produce the
> same results as on i386.
>
> Is this form okay?

You should pick different names to avoid possible conflicts.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Hilko Bengen]

* Alexandre Julliard:
> > Is this form okay?
> > You should pick different names to avoid possible conflicts.

Please have a look at the revised patch I just posted. I have avoided
possible conflicts by only defining the macros if they are needed.

-Hilko

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Alexandre Julliard]

Hilko Bengen <bengen(a)hilluzination.de> writes:
> * Alexandre Julliard:
> > > Is this form okay?
> > > You should pick different names to avoid possible conflicts.
>
> Please have a look at the revised patch I just posted. I have avoided
> possible conflicts by only defining the macros if they are needed.

A different name is much better.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[Hilko Bengen]

* Alexandre Julliard:
> > Please have a look at the revised patch I just posted. I have avoided
> > possible conflicts by only defining the macros if they are needed.
>
> A different name is much better.

Alright. Any suggestions?

-Hilko

[PATCH] sfnt2fnt: Fix broken .fon files on !x86 architectures

[David Laight]

On Fri, Jun 15, 2012 at 07:52:50PM +0200, Hilko Bengen wrote:
> +#define htole16(_x) ( (((_x) & 0xff) << 8) |            \
> +                      (((_x) & 0xff00) >> 8) )
> +#define htole32(_x) ( (((_x) & 0xff) << 24) |           \
> +                      (((_x) & 0xff00) << 8) |          \
> +                      (((_x) & 0xff0000) >> 8) |        \
> +                      (((_x) & 0xff000000) >> 24) )

You shouldn't really define macros that expand their arguments
multiple times.

	David

-- 
David Laight: david(a)l8s.co.uk