This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Make Stap compatible with Android


I took a quick look at your patch. Comments:

1) build_android.sh: That script seems quite specific to your setup
(your choice of BUILD_PREFIX and OUTPUT_DIR for instance). I think I
would suggest we put something like that on the systemtap wiki in an
article about building for android, not in the source tree.

2) I think there's a typo around line 116 in your patch. You've got:

====
@@ -580,10 +587,14 @@ AC_MSG_NOTICE([stap will link $stap_LIBS])
 if test $build_elfutils = no; then
   save_LIBS="$LIBS"
   dnl this will only succeed with elfutils 0.142+
-  AC_CHECK_LIB(elf,elf_getshdrstrndx,[],[
-    AC_MSG_FAILURE([libelf too old, need 0.142+])])
-  staprun_LIBS="$staprun_LIBS -lelf"
-  stapbpf_LIBS="$stapbpf_LIBS -lelf"
+  if test "${have_android}" == "bo"; then
+ AC_CHECK_LIB(elf,elf_getshdrstrndx,
+ [staprun_LIBS="$staprun_LIBS -lelf"
+ LIBS="$save_LIBS"],
+ [AC_MSG_FAILURE([libelf too old, need 0.142+])])
+ staprun_LIBS="$staprun_LIBS -lelf"
+ stapbpf_LIBS="$stapbpf_LIBS -lelf"
+  fi
====

Shouldn't that test be checking for the string "no", not "bo"? I also
don't understand why you changed the original code. I understand
surrounding it with the 'if have_android == "no"' test, but not the
changes themselves. I would have expected just to see whitespace
changes to the original code there.

3) I'm also not sure about the approach you are taking here. You seem
to assume that android will *never* have an elfutils 0.142+. Why don't
you test the elfutils version and change the configure macro from
HAVE_ANDROID to something like HAVE_ANDROID_OLD_ELFUTILS (feel free to
use a better name) if the elfutils version isn't new enough?


On Mon, Apr 30, 2018 at 5:44 AM, Alexander Lochmann
<alexander.lochmann@tu-dortmund.de> wrote:
> Hi folks,
>
> it's been a long time since my first approach to submit my Android patch
> set.
> I re-worked my code 'a bit' and stripped it down to the bare essentials.
> The result is attached to this mail. The patch is based on release-3.2.
> It basically adds a new configure options: --with-android. This option
> makes the configure libelf check optional and disables some parts of
> stap{run,io} which depend on libelf.
> The result is two binaries, staprun and stapio, which can be executed on
> Android.
>
> What do you think about these changes?
>
> Regards,
> Alex
>
> --
> Technische Universität Dortmund
> Alexander Lochmann                PGP key: 0xBC3EF6FD
> Otto-Hahn-Str. 16                 phone:  +49.231.7556141
> D-44227 Dortmund                  fax:    +49.231.7556116
> http://ess.cs.tu-dortmund.de/Staff/al



-- 
David Smith
Associate Manager
Red Hat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]