GNOME Bugzilla – Bug 707161
[Patch] Port gssdp to Android
Last modified: 2019-02-22 09:29:42 UTC
Created attachment 253667 [details] [review] patch Hello Attached patch allows you to build and use gssdp on Android. Tested with Android's NDK r8e but should work with earlier versions too. Testing device was a Galaxy Nexus running 4.2/4.3. There wasn't that much work missing to be able to bring gssdp to Android (congrats on a great work ;) only missing bit was to avoid relying on iffaddrs.h , which is not available on the platform. The attached patch just adds a netlink fallback for get_host_ip () then. Please review and comment so I can make any needed amendments. Patch should apply cleanly to .14.2 .14.4 and today's master. I left a couple of log messages around so you can easily test it on Android (As without some tricks you wont get stdout/stderr when using gssdp from an Application) but I can of course drop them if you don't like the idea. To give you more context; Intel's OTC is working on bringing an opensource DLNA stack to Android. This small enhancement is part of that effort.
Review of attachment 253667 [details] [review]: - Could we use the android code on generic linux as well? - With the third platform I think we should start splitting the implementation into conditionally compiled files rather than #ifdefing in the file ::: libgssdp/gssdp-client.c @@ +61,3 @@ +#include <net/if.h> +#include <stdlib.h> +//#include <linux/netlink.h> No commented code, please :) @@ +1264,3 @@ + + if ((sock = socket (AF_INET, SOCK_STREAM, 0)) < 0) { + struct sockaddr_in *address, *netmask; Please align parameters as per coding guideline. @@ +1281,3 @@ + ifconfigs.ifc_buf = (char *) ifaces; + + return FALSE; We currently only do IPv4, so this is fine currently. This needs revisiting when we address bug 654137
Created attachment 253766 [details] [review] Updated patch, try2
(In reply to comment #1) > Review of attachment 253667 [details] [review]: > > - Could we use the android code on generic linux as well? It should be possible yes, but that will require also to either ifdef the debug messages against __BIONIC__ for the __android_log_() calls or maybe wrap around all log messages and have, say, gssdp_log() with separate implementations for *nix/android/etc. I think it's a good idea nonetheless but maybe better pursued as a follow up enhancement? > - With the third platform I think we should start splitting the implementation > into conditionally compiled files rather than #ifdefing in the file I agree but as I pointed in the above comment/question, I think this may be better handled separately. I hereby volunteer to take care of that once we get this upstreamed, deal? ;) > ::: libgssdp/gssdp-client.c > @@ +61,3 @@ > +#include <net/if.h> > +#include <stdlib.h> > +//#include <linux/netlink.h> > > No commented code, please :) Leftover removed. > @@ +1264,3 @@ > + > + if ((sock = socket (AF_INET, SOCK_STREAM, 0)) < 0) { > + struct sockaddr_in *address, *netmask; > > Please align parameters as per coding guideline. I corrected this there and on some other similar lines but if the indentation change required breaking an user visible string I didn't. I usually do this to avoid harming an user's availability to grep for a given error/debug message in the code. Now, if you'd rather have me chopping every too large "string" down to follow the coding style I can still do it, whatever you prefer really. I'm referring to these kind of occurrences btw: __android_log_print (ANDROID_LOG_DEBUG, "gssdp", "Got list of %d interfaces. Looking for a suitable one", if_num); > @@ +1281,3 @@ > + ifconfigs.ifc_buf = (char *) ifaces; > + > + return FALSE; > > We currently only do IPv4, so this is fine currently. This needs revisiting > when we address bug 654137 Yeah, noticed but forgot rephrasing the comment. Updated now. Please find these changes in the attached try2 patch attached. Thanks a lot for your quick review.