Bug 95481 - Build fails on Android due to broken lconv struct
Summary: Build fails on Android due to broken lconv struct
Status: RESOLVED FIXED
Alias: None
Product: fontconfig
Classification: Unclassified
Component: library (show other bugs)
Version: 2.11
Hardware: All other
: medium major
Assignee: Akira TAGOH
QA Contact: Behdad Esfahbod
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-19 02:44 UTC by Rodger Combs
Modified: 2016-05-27 02:17 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Rodger Combs 2016-05-19 02:44:06 UTC
This breaks FcStrtod. I'd suggest one of two solutions:
1. Remove the locale check and rely on the fontconfig-provided strtod implementation in all cases, rather than using the libc one only if the decimal_point is ".". This seems preferable in general, as it prevents behavior from potentially differing based on locale.
2. Add a configure check for a working lconv struct containing a decimal_point field, and #ifdef out the check if it's not present. This maintains current behavior on locales where the decimal point is ".", but adds complexity, and any case where it causes problems would cause the same problems in other locales anyway.
Comment 1 Akira TAGOH 2016-05-26 05:26:22 UTC
Taking a look at the original code based on FcStrtod(), i.e. glib's code and I see they do ifdef with __BIONIC__ which was introduced to fix an issue on Android. does this patch work?

diff --git a/src/fcxml.c b/src/fcxml.c
index cd8fff1..f9b5975 100644
--- a/src/fcxml.c
+++ b/src/fcxml.c
@@ -1352,7 +1352,11 @@ FcParseInt (FcConfigParse *parse)
 static double
 FcStrtod (char *s, char **end)
 {
+#ifndef __BIONIC__
     struct lconv    *locale_data;
+#endif
+    const char	    *decimal_point;
+    int		    dlen;
     char	    *dot;
     double	    v;
 
@@ -1360,14 +1364,21 @@ FcStrtod (char *s, char **end)
      * Have to swap the decimal point to match the current locale
      * if that locale doesn't use 0x2e
      */
+#ifndef __BIONIC__
+    locale_data = localeconv ();
+    decimal_point = locale_data->decimal_point;
+    dlen = strlen (decimal_point);
+#else
+    decimal_point = ".";
+    dlen = 1;
+#endif
+
     if ((dot = strchr (s, 0x2e)) &&
-	(locale_data = localeconv ()) &&
-	(locale_data->decimal_point[0] != 0x2e ||
-	 locale_data->decimal_point[1] != 0))
+	(decimal_point[0] != 0x2e ||
+	 decimal_point[1] != 0))
     {
 	char	buf[128];
 	int	slen = strlen (s);
-	int	dlen = strlen (locale_data->decimal_point);
 	
 	if (slen + dlen > (int) sizeof (buf))
 	{
Comment 2 Akira TAGOH 2016-05-26 05:28:31 UTC
(In reply to Akira TAGOH from comment #1)
> Taking a look at the original code based on FcStrtod(), i.e. glib's code and

meant "Taking a look at the original code of FcStrtod() borrowing from"
Comment 3 Rodger Combs 2016-05-27 01:40:25 UTC
There's still a use of locale_data later in the function:

fcxml.c: In function 'FcStrtod':
fcxml.c:1388:31: error: 'locale_data' undeclared (first use in this function)
      strcpy (buf + (dot - s), locale_data->decimal_point);
                               ^
fcxml.c:1388:31: note: each undeclared identifier is reported only once for each function it appears in


I'd imagine you'll just want to change that to use the local `decimal_point` variable. Otherwise, looks fine.
Comment 4 Akira TAGOH 2016-05-27 02:17:20 UTC
Ah, yes. you're right.

Okay, committed the change in git after replacing one more locale_data thing.
Thanks


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.