| Message ID | 20170419110155.6828-4-arkadiusz.hiler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 19 Apr 2017, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote: > We define gettid() using syscall() because glibc does not provide a > wrapper. > > Android's bionic got the syscall covered though. > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > --- > lib/igt_aux.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/lib/igt_aux.h b/lib/igt_aux.h > index e62858e..54b9716 100644 > --- a/lib/igt_aux.h > +++ b/lib/igt_aux.h > @@ -43,7 +43,12 @@ extern int num_trash_bos; > #define NSEC_PER_SEC (1000*USEC_PER_SEC) > > /* signal interrupt helpers */ > +#ifdef ANDROID Seems like this should be something like HAVE_GETTID, defined by configure or by android makefiles? BR, Jani. > +#include <unistd.h> /* on Android bionic has this implemented */ > +#else > #define gettid() syscall(__NR_gettid) > +#endif > + > #define sigev_notify_thread_id _sigev_un._tid > > /* auxialiary igt helpers from igt_aux.c */
On Wed, Apr 19, 2017 at 03:22:19PM +0300, Jani Nikula wrote: > On Wed, 19 Apr 2017, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote: > > We define gettid() using syscall() because glibc does not provide a > > wrapper. > > > > Android's bionic got the syscall covered though. > > > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > > --- > > lib/igt_aux.h | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/lib/igt_aux.h b/lib/igt_aux.h > > index e62858e..54b9716 100644 > > --- a/lib/igt_aux.h > > +++ b/lib/igt_aux.h > > @@ -43,7 +43,12 @@ extern int num_trash_bos; > > #define NSEC_PER_SEC (1000*USEC_PER_SEC) > > > > /* signal interrupt helpers */ > > +#ifdef ANDROID > > Seems like this should be something like HAVE_GETTID, defined by > configure or by android makefiles? Yeah, but that's not that easy (yet) and that's not the only area which would use it - notice the thing with cairo from the cover letter. config.h is generated in a ugly way for Android - lib/Android.mk does that. Also if you ./configure it stops compiling for Android causing confusing error. Whole area could use some heavy reworking. One approach would be to mimic what other projects do: * have config_android.h with set of sane #defines (as environment is more static and there is no ac/am) * don't define HAVE_CONFIG_H and just `-include config_android.h` on... Android * add gettid() discovery via ac for Linux (I don't think that any libc other than bionic wraps that call though) But that would made a whole series. I would like to go with #ifdef ANDROID for now.
On Wed, 19 Apr 2017, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote: > On Wed, Apr 19, 2017 at 03:22:19PM +0300, Jani Nikula wrote: >> On Wed, 19 Apr 2017, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote: >> > We define gettid() using syscall() because glibc does not provide a >> > wrapper. >> > >> > Android's bionic got the syscall covered though. >> > >> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> >> > --- >> > lib/igt_aux.h | 5 +++++ >> > 1 file changed, 5 insertions(+) >> > >> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h >> > index e62858e..54b9716 100644 >> > --- a/lib/igt_aux.h >> > +++ b/lib/igt_aux.h >> > @@ -43,7 +43,12 @@ extern int num_trash_bos; >> > #define NSEC_PER_SEC (1000*USEC_PER_SEC) >> > >> > /* signal interrupt helpers */ >> > +#ifdef ANDROID >> >> Seems like this should be something like HAVE_GETTID, defined by >> configure or by android makefiles? > > Yeah, but that's not that easy (yet) and that's not the only area which > would use it - notice the thing with cairo from the cover letter. > > config.h is generated in a ugly way for Android - lib/Android.mk does > that. Also if you ./configure it stops compiling for Android causing > confusing error. > > Whole area could use some heavy reworking. > > One approach would be to mimic what other projects do: > > * have config_android.h with set of sane #defines (as environment is > more static and there is no ac/am) > > * don't define HAVE_CONFIG_H and just `-include config_android.h` on... > Android > > * add gettid() discovery via ac for Linux (I don't think that any > libc other than bionic wraps that call though) > > > But that would made a whole series. > I would like to go with #ifdef ANDROID for now. Fair enough. It's just that I cringe seeing #ifdef <ENVIRONMENT> instead of #ifdef <FEATURE>, when <ENVIRONMENT> and <FEATURE> are not interchangeable or 1:1. For example, glibc might include gettid later. BR, Jani.
On Wed, Apr 19, 2017 at 05:23:46PM +0300, Jani Nikula wrote: > On Wed, 19 Apr 2017, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote: > > On Wed, Apr 19, 2017 at 03:22:19PM +0300, Jani Nikula wrote: > >> On Wed, 19 Apr 2017, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote: > >> > We define gettid() using syscall() because glibc does not provide a > >> > wrapper. > >> > > >> > Android's bionic got the syscall covered though. > >> > > >> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > >> > --- > >> > lib/igt_aux.h | 5 +++++ > >> > 1 file changed, 5 insertions(+) > >> > > >> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h > >> > index e62858e..54b9716 100644 > >> > --- a/lib/igt_aux.h > >> > +++ b/lib/igt_aux.h > >> > @@ -43,7 +43,12 @@ extern int num_trash_bos; > >> > #define NSEC_PER_SEC (1000*USEC_PER_SEC) > >> > > >> > /* signal interrupt helpers */ > >> > +#ifdef ANDROID > >> > >> Seems like this should be something like HAVE_GETTID, defined by > >> configure or by android makefiles? > > > > Yeah, but that's not that easy (yet) and that's not the only area which > > would use it - notice the thing with cairo from the cover letter. > > > > config.h is generated in a ugly way for Android - lib/Android.mk does > > that. Also if you ./configure it stops compiling for Android causing > > confusing error. > > > > Whole area could use some heavy reworking. > > > > One approach would be to mimic what other projects do: > > > > * have config_android.h with set of sane #defines (as environment is > > more static and there is no ac/am) > > > > * don't define HAVE_CONFIG_H and just `-include config_android.h` on... > > Android > > > > * add gettid() discovery via ac for Linux (I don't think that any > > libc other than bionic wraps that call though) > > > > > > But that would made a whole series. > > I would like to go with #ifdef ANDROID for now. > > Fair enough. > > It's just that I cringe seeing #ifdef <ENVIRONMENT> instead of #ifdef > <FEATURE>, when <ENVIRONMENT> and <FEATURE> are not interchangeable or > 1:1. For example, glibc might include gettid later. > > BR, > Jani. I completetly get that, I had mixed feeling adding the ifdef.
diff --git a/lib/igt_aux.h b/lib/igt_aux.h index e62858e..54b9716 100644 --- a/lib/igt_aux.h +++ b/lib/igt_aux.h @@ -43,7 +43,12 @@ extern int num_trash_bos; #define NSEC_PER_SEC (1000*USEC_PER_SEC) /* signal interrupt helpers */ +#ifdef ANDROID +#include <unistd.h> /* on Android bionic has this implemented */ +#else #define gettid() syscall(__NR_gettid) +#endif + #define sigev_notify_thread_id _sigev_un._tid /* auxialiary igt helpers from igt_aux.c */
We define gettid() using syscall() because glibc does not provide a wrapper. Android's bionic got the syscall covered though. Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com> --- lib/igt_aux.h | 5 +++++ 1 file changed, 5 insertions(+)