After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 689223 - Fix compilation on Android with the bionic C library
Fix compilation on Android with the bionic C library
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: build
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
android
Depends on:
Blocks:
 
 
Reported: 2012-11-28 15:30 UTC by Sebastian Dröge (slomo)
Modified: 2013-04-16 11:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix compilation on Android with the bionic C library (10.80 KB, patch)
2012-11-28 15:31 UTC, Sebastian Dröge (slomo)
none Details | Review
Fix compilation on Android with the bionic C library (20.58 KB, patch)
2013-03-03 10:43 UTC, Sebastian Dröge (slomo)
none Details | Review
Fix compilation on Android with the bionic C library (20.30 KB, patch)
2013-04-16 08:21 UTC, Sebastian Dröge (slomo)
reviewed Details | Review

Description Sebastian Dröge (slomo) 2012-11-28 15:30:46 UTC
Hi,

the attached patch fixes the compilation of GLib on Android with the bionic C library that is used there by default.

Unfortunately some parts of the API used by the resolver objects is not public in the Android NDK (yet) so it is copied from the private headers for now. I don't expect any problems here in the future because Android is using a plain copy of the NetBSD network stack.


This is the patch that is used in the GStreamer SDK for Android.
Comment 1 Sebastian Dröge (slomo) 2012-11-28 15:31:09 UTC
Created attachment 230094 [details] [review]
Fix compilation on Android with the bionic C library
Comment 2 Sebastian Dröge (slomo) 2012-11-28 15:38:38 UTC
Hrm, I accidentially pushed this change instead of another one now. Shall I revert?
Comment 3 Dan Winship 2012-11-28 15:39:35 UTC
yes
Comment 4 Sebastian Dröge (slomo) 2012-11-28 15:56:43 UTC
Done, sorry for that
Comment 5 Sebastian Dröge (slomo) 2013-03-03 10:43:44 UTC
Created attachment 237869 [details] [review]
Fix compilation on Android with the bionic C library
Comment 6 Sebastian Dröge (slomo) 2013-03-03 10:44:24 UTC
Comment on attachment 230094 [details] [review]
Fix compilation on Android with the bionic C library

The new version now applies cleanly and builds with latest git master as of today.
Comment 7 Sebastian Dröge (slomo) 2013-04-16 08:21:59 UTC
Created attachment 241623 [details] [review]
Fix compilation on Android with the bionic C library
Comment 8 Sebastian Dröge (slomo) 2013-04-16 08:24:39 UTC
Would be great to get at least an initial review here, or someone saying that you're simply not interested in this for GLib.
Comment 9 Colin Walters 2013-04-16 09:09:55 UTC
Review of attachment 241623 [details] [review]:

Minor comments, looks good to me in general.

::: configure.ac
@@ +1028,3 @@
+dnl If int/long are the same size, we see which one produces
+dnl warnings when used in the location as ssize_t. (This matters
+dnl on Android where ssize_t is long and size_t is unsigned int)

That's...just crazy.

::: gio/glocalfileinfo.c
@@ +1109,3 @@
 	  data->real_name = convert_pwd_string_to_utf8 (gecos);
 	}
+#endif

Is there maybe some stub data we could return here rather than just silently doing nothing?  Username/group "root"?

::: gio/gresolver.c
@@ +248,2 @@
           res_init ();
+#endif

AC_CHECK_FUNCS(res_init) and #ifdef HAVE_RES_INIT ?
Comment 10 Sebastian Dröge (slomo) 2013-04-16 10:45:17 UTC
(In reply to comment #9)
> Review of attachment 241623 [details] [review]:
> 
> Minor comments, looks good to me in general.
> 
> ::: configure.ac
> @@ +1028,3 @@
> +dnl If int/long are the same size, we see which one produces
> +dnl warnings when used in the location as ssize_t. (This matters
> +dnl on Android where ssize_t is long and size_t is unsigned int)
> 
> That's...just crazy.

Yes, what did you expect from Google? :)

> ::: gio/glocalfileinfo.c
> @@ +1109,3 @@
>        data->real_name = convert_pwd_string_to_utf8 (gecos);
>      }
> +#endif
> 
> Is there maybe some stub data we could return here rather than just silently
> doing nothing?  Username/group "root"?

I don't know, I would be fine with anything there :)
What do you want to there?

> ::: gio/gresolver.c
> @@ +248,2 @@
>            res_init ();
> +#endif
> 
> AC_CHECK_FUNCS(res_init) and #ifdef HAVE_RES_INIT ?

Yes, will change it accordingly for the next version of the patch.


Is it ready to be pushed after the two changes above or would you want another review by someone else?
Comment 11 Colin Walters 2013-04-16 10:58:47 UTC
(In reply to comment #10)

> I don't know, I would be fine with anything there :)
> What do you want to there?

It just seems possible to me that there's GLib programs out there that query for owner::user and aren't prepared for it to be NULL, because GLib will even synthesize:

  data->real_name = g_strdup_printf ("user #%d", (int)uid);

if GECOS lookup fails.  So we could just stick in "android" or something; dunno.

> Is it ready to be pushed after the two changes above or would you want another
> review by someone else?

Dunno, it seems safe and sane enough to me, but Ryan would be the other likely person to sign off if you want to wait for that.  We can always fix up things later too.
Comment 12 Sebastian Dröge (slomo) 2013-04-16 11:12:09 UTC
(In reply to comment #11)
> (In reply to comment #10)
> 
> > I don't know, I would be fine with anything there :)
> > What do you want to there?
> 
> It just seems possible to me that there's GLib programs out there that query
> for owner::user and aren't prepared for it to be NULL, because GLib will even
> synthesize:
> 
>   data->real_name = g_strdup_printf ("user #%d", (int)uid);
> 
> if GECOS lookup fails.  So we could just stick in "android" or something;
> dunno.

It's doing that some lines below already if everything failed:

      if (!e.user_name)
        e.user_name = g_strdup ("somebody");
      if (!e.real_name)
        e.real_name = g_strdup ("Unknown");
Comment 13 Sebastian Dröge (slomo) 2013-04-16 11:26:32 UTC
Ok, pushed with the res_init() change. Let's fix anything else that shows up later then :)

Many thanks for the review!