Fontconfig - May 2012 - [PATCH 1/2] Extra ', ' in AC_ARG_WITH(arch causes arch to never be autodetected

Commit 87d7b82a98780223422a829b6bb1a05fd753ae5e reformatted this
part of the configure script, accidentally introducing a spurious
comma.

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 configure.in |    1 -
 1 file changed, 1 deletion(-)

diff --git a/configure.in b/configure.in
index b77c52a..b2174d9 100644
--- a/configure.in
+++ b/configure.in
@@ -110,7 +110,6 @@ dnl
=========================================================================
AC_ARG_WITH(arch,
 	[AC_HELP_STRING([--with-arch=ARCH],
 			[Force architecture to ARCH])],
-	,
 	arch="$withval", arch=auto)
 
 if test "x$arch" != xauto; then
-- 
1.7.10

This patch isn''t really tested as I don''t have such a machine,
but I
have a bug report that on m68k machines, double values are aligned on
smaller than 4 byte boundaries. If ALIGNOF_DOUBLE < sizeof(int),
the "expected" sizeof of FcValue is miscomputed. Use the maximum of 4
(sizeof (int)) and ALIGNOF_DOUBLE when computing the expected size of
FcValue.

Signed-off-by: Keith Packard <keithp at keithp.com>
---
 src/fcarch.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/fcarch.c b/src/fcarch.c
index 09d24b3..5fe7d97 100644
--- a/src/fcarch.c
+++ b/src/fcarch.c
@@ -56,7 +56,9 @@ FC_ASSERT_STATIC (SIZEOF_VOID_P == sizeof (FcStrSet *));
 FC_ASSERT_STATIC (SIZEOF_VOID_P == sizeof (FcCharLeaf **));
 FC_ASSERT_STATIC (SIZEOF_VOID_P == sizeof (FcChar16 *));
 
-FC_ASSERT_STATIC (0x08 + 1*ALIGNOF_DOUBLE == sizeof (FcValue));
+#define FC_MAX(a,b)	((a) > (b) ? (a) : (b))
+
+FC_ASSERT_STATIC (0x08 + 1*FC_MAX(4,ALIGNOF_DOUBLE) == sizeof (FcValue));
 FC_ASSERT_STATIC (0x00 + 2*SIZEOF_VOID_P == sizeof (FcPatternElt));
 FC_ASSERT_STATIC (0x08 + 2*SIZEOF_VOID_P == sizeof (FcPattern));
 FC_ASSERT_STATIC (0x08 + 2*SIZEOF_VOID_P == sizeof (FcCharSet));
-- 
1.7.10

Doh. thanks for catching this up.

On Wed, May 2, 2012 at 11:28 AM, Keith Packard <keithp at keithp.com>
wrote:
> Commit 87d7b82a98780223422a829b6bb1a05fd753ae5e reformatted this
> part of the configure script, accidentally introducing a spurious
> comma.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
> ?configure.in | ? ?1 -
> ?1 file changed, 1 deletion(-)
>
> diff --git a/configure.in b/configure.in
> index b77c52a..b2174d9 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -110,7 +110,6 @@ dnl
=========================================================================>
?AC_ARG_WITH(arch,
> ? ? ? ?[AC_HELP_STRING([--with-arch=ARCH],
> ? ? ? ? ? ? ? ? ? ? ? ?[Force architecture to ARCH])],
> - ? ? ? ,
> ? ? ? ?arch="$withval", arch=auto)
>
> ?if test "x$arch" != xauto; then
> --
> 1.7.10
>


-- 
Akira TAGOH

That looks good to me. thanks!

On Wed, May 2, 2012 at 11:28 AM, Keith Packard <keithp at keithp.com>
wrote:
> This patch isn''t really tested as I don''t have such a
machine, but I
> have a bug report that on m68k machines, double values are aligned on
> smaller than 4 byte boundaries. If ALIGNOF_DOUBLE < sizeof(int),
> the "expected" sizeof of FcValue is miscomputed. Use the maximum
of 4
> (sizeof (int)) and ALIGNOF_DOUBLE when computing the expected size of
> FcValue.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
> ?src/fcarch.c | ? ?4 +++-
> ?1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/fcarch.c b/src/fcarch.c
> index 09d24b3..5fe7d97 100644
> --- a/src/fcarch.c
> +++ b/src/fcarch.c
> @@ -56,7 +56,9 @@ FC_ASSERT_STATIC (SIZEOF_VOID_P == sizeof (FcStrSet *));
> ?FC_ASSERT_STATIC (SIZEOF_VOID_P == sizeof (FcCharLeaf **));
> ?FC_ASSERT_STATIC (SIZEOF_VOID_P == sizeof (FcChar16 *));
>
> -FC_ASSERT_STATIC (0x08 + 1*ALIGNOF_DOUBLE == sizeof (FcValue));
> +#define FC_MAX(a,b) ? ?((a) > (b) ? (a) : (b))
> +
> +FC_ASSERT_STATIC (0x08 + 1*FC_MAX(4,ALIGNOF_DOUBLE) == sizeof (FcValue));
> ?FC_ASSERT_STATIC (0x00 + 2*SIZEOF_VOID_P == sizeof (FcPatternElt));
> ?FC_ASSERT_STATIC (0x08 + 2*SIZEOF_VOID_P == sizeof (FcPattern));
> ?FC_ASSERT_STATIC (0x08 + 2*SIZEOF_VOID_P == sizeof (FcCharSet));
> --
> 1.7.10
>


-- 
Akira TAGOH

On Wed, 2 May 2012 12:05:45 +0900, Akira TAGOH <akira at tagoh.org> wrote:
> That looks good to me. thanks!
I''ll wait to make sure this actually fixes the bug reported on m68k and
reply with that info.

btw, what''s the commit process for fontconfig these days? Are you
merging patches and pushing them to master? Or are we still all pushing
on our own?

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/fontconfig/attachments/20120501/e4ec1476/attachment.pgp>

On Wed, May 2, 2012 at 12:14 PM, Keith Packard <keithp at keithp.com>
wrote:
> I''ll wait to make sure this actually fixes the bug reported on
m68k and
> reply with that info.
Sure.
> btw, what''s the commit process for fontconfig these days? Are you
> merging patches and pushing them to master? Or are we still all pushing
> on our own?
Yes, fd.o''s is still master tree for fontconfig. though I have own
tree on people.fd.o too to make aggressive changes or to manage fixes
on bz. once it''s ready to go, I''m merging changes into master.
that
would gives us opportunities to review since the commit logs are sent
to this list. so please go ahead if any.

-- 
Akira TAGOH

On Tue, 01 May 2012 20:14:52 -0700, Keith Packard <keithp at keithp.com>
wrote:
> I''ll wait to make sure this actually fixes the bug reported on
m68k and
> reply with that info.
Looks like it actually worked. Thanks for merging it to master.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/fontconfig/attachments/20120502/0b6da2a2/attachment.pgp>