On Wed, Aug 23, 2017 at 09:47:22PM +0200, Eugene Syromiatnikov wrote: > > * strace.c (alloctcb): update the condition of presence of currpers > > field. > Btw, looks like there are other places that touch that field, wouldn't > it better to provide setter/getter for this field which would resolve to > null function in case no currpers field present? Otherwise it's quite > difficult to maintain currpers-touching code and decide which macros > should be checked in each case. > The rule is simple: places others than its initialization should not touch it at all, in case of defined(USE_LUAJIT) && SUPPORTED_PERSONALITIES == 1. So they should only check for SUPPORTED_PERSONALITIES > 1, as they did before.
> > diff --git a/defs_shared.h b/defs_shared.h > > new file mode 100644 > > index 00000000..1fb7a92d > > --- /dev/null > > +++ b/defs_shared.h > > @@ -0,0 +1,66 @@ > > +/* > > + * Should only be included without FFI_CDEF from defs.h, so no include > > guards. > > + */ > BTW, you can add something like > > #if !defined(FFI_CDEF) || !defined(STRACE_DEFS_SHARED_H) > #ifndef FFI_CDEF > # define STRACE_DEFS_SHARED_H > #endif /* !FFI_CDEF */ > > <...> > > #endif /* !FFI_CDEF || !STRACE_DEFS_SHARED_H */ > > to avoid commenting about absence of include guards. > Well, defs_shared.h is not a proper header file in that it does not include anything it needs, and such include guards would imply it can be included from elsewhere. > > diff --git a/luajit.h b/luajit.h > > new file mode 100644 > > index 00000000..db8a9474 > > --- /dev/null > > +++ b/luajit.h > > @@ -0,0 +1,311 @@ > > +/* > > + * Should only be included from strace.c, so no include guards. > > + */ > Same here, regarding include guard. luajit.h is not a proper header file either, in that it 1) does not include anything it needs; and 2) contains (non-static inline) functions. Again, such include guards would imply it can be included from elsewhere. > The implementation variant of the FFI library used looks like build-time > choice > and I can't see any difference in the interface of these library versions, > is the library with which strace is built important for the strace users? > You may want to elaborate on this, however, in configure.ac or install.texi. strace is built with a Lua implementation, not a FFI library. LuaJIT comes with its own FFI library, non-LuaJIT users have to install a standalone one. A compile-time difference is that LuaJIT defines LUA_FFILIBNAME, and, in absence of it, strace tries to load a library named "ffi". The only run-time difference important to us is how a comparison against a null pointer should be done: LuaJIT requires comparing a boxed pointer against nil; luaffi with ffi.C.NULL; and luaffifb with ffi.NULL. Oh, and Lua 5.1 (non-LuaJIT) is not supported as for now, due to that standalone implementations don't support arithmetics on pointers. Honestly, I am not sure if supporting non-LuaJIT is worth it. > So, the script as simple as > > tcp = strace.C.next_sc() > print(tcp.scno) > > would crash strace process itself. What do you think about protecting > against those simplest ways to crash strace? Dunno. Put a warning into the man page? ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel