[PATCH] asinfo: Introduce static query tool asinfo

Edgar Kaziahmedov edos at linux.com
Tue Jun 20 16:25:33 UTC 2017


On Mon, 19 Jun 2017 08:30:26 +0200
Eugene Syromiatnikov <esyr at redhat.com> wrote:

> On Thu, Jun 15, 2017 at 03:14:05PM +0300, Edgar Kaziahmedov wrote:
> > Main features were described in the status report,  
> It's better to reiterate the description in the commit message, as the
> phrase "described in the status report" would be of little value in
> several years from now.
> 
> > for example now, tool can print out list of archs with
> > total number of syscalls.  
> nsyscalls != number of syscalls.
> 
> > 
> > $ ./tools/asinfo/asinfo
> >      arm64 : 1080
> >    aarch64 : 1080
> >        arm : 453
> >     x86_64 : 333
> >        x32 : 548
> > 
> > * Makefile.am: Add subdir
> > * configure.ac: Add new binary
> > * tools/Makefile.am: New binary
> > * tools/asinfo/Makefile.am: New binary
> > * tools/asinfo/arch_list.h: List of architectures, in the future
> > this file could be generated using some script
> > * tools/asinfo/asinfo.*: Example of using disparchers.c libasinfo
> > * tools/asinfo/dispatchers.*: All dispacthers are placed here
> > * tools/asinfo/request_msgs.h: List of input parameters and
> > structures for syscall_dispacther
> > * tools/asinfo/xshortmalloc.c: Complementary file to work with
> > memory
> > 
> > It's the draft commit to assess the direction of development
> > 
> > Signed-off-by: Edgar Kaziahmedov <edos at linux.com>
> > ---
> >  Makefile.am                 |   2 +-
> >  configure.ac                |   2 +
> >  tools/Makefile.am           |  28 ++++++++
> >  tools/asinfo/Makefile.am    |  57 ++++++++++++++++
> >  tools/asinfo/arch_list.h    |   5 ++
> >  tools/asinfo/asinfo.1       |   0
> >  tools/asinfo/asinfo.c       |  96 ++++++++++++++++++++++++++
> >  tools/asinfo/asinfo.h       |  95 ++++++++++++++++++++++++++
> >  tools/asinfo/dispatchers.c  | 159
> > ++++++++++++++++++++++++++++++++++++++++++++
> > tools/asinfo/dispatchers.h  |  99 +++++++++++++++++++++++++++
> > tools/asinfo/request_msgs.h |  72 ++++++++++++++++++++
> > tools/asinfo/xshortmalloc.c |  59 ++++++++++++++++ 12 files
> > changed, 673 insertions(+), 1 deletion(-) create mode 100644
> > tools/Makefile.am create mode 100644 tools/asinfo/Makefile.am
> >  create mode 100644 tools/asinfo/arch_list.h
> >  create mode 100644 tools/asinfo/asinfo.1
> >  create mode 100644 tools/asinfo/asinfo.c
> >  create mode 100644 tools/asinfo/asinfo.h
> >  create mode 100644 tools/asinfo/dispatchers.c
> >  create mode 100644 tools/asinfo/dispatchers.h
> >  create mode 100644 tools/asinfo/request_msgs.h
> >  create mode 100644 tools/asinfo/xshortmalloc.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 3d6eba1e..665ac915 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -35,7 +35,7 @@ endif
> >  if HAVE_MX32_RUNTIME
> >  TESTS_MX32 = tests-mx32
> >  endif
> > -SUBDIRS = tests $(TESTS_M32) $(TESTS_MX32)
> > +SUBDIRS = tests $(TESTS_M32) $(TESTS_MX32) tools
> >  
> >  bin_PROGRAMS = strace
> >  man_MANS = strace.1
> > diff --git a/configure.ac b/configure.ac
> > index 0d4679e1..e0817aa5 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -822,6 +822,8 @@ AC_CONFIG_FILES([Makefile
> >  		 tests/Makefile
> >  		 tests-m32/Makefile
> >  		 tests-mx32/Makefile
> > +		 tools/Makefile
> > +		 tools/asinfo/Makefile
> >  		 strace.spec
> >  		 debian/changelog])
> >  AC_OUTPUT
> > diff --git a/tools/Makefile.am b/tools/Makefile.am
> > new file mode 100644
> > index 00000000..6044e9ae
> > --- /dev/null
> > +++ b/tools/Makefile.am
> > @@ -0,0 +1,28 @@
> > +# Automake input for strace tools.
> > +#
> > +# Copyright (c) 2017 Edgar A. Kaziakhmedov <edos at linux.com>
> > +# All rights reserved.
> > +#
> > +# Redistribution and use in source and binary forms, with or
> > without +# modification, are permitted provided that the following
> > conditions +# are met:
> > +# 1. Redistributions of source code must retain the above copyright
> > +#    notice, this list of conditions and the following disclaimer.
> > +# 2. Redistributions in binary form must reproduce the above
> > copyright +#    notice, this list of conditions and the following
> > disclaimer in the +#    documentation and/or other materials
> > provided with the distribution. +# 3. The name of the author may
> > not be used to endorse or promote products +#    derived from this
> > software without specific prior written permission. +#
> > +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR +# IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> > THE IMPLIED WARRANTIES +# OF MERCHANTABILITY AND FITNESS FOR A
> > PARTICULAR PURPOSE ARE DISCLAIMED. +# IN NO EVENT SHALL THE AUTHOR
> > BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL,
> > EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT +# NOT LIMITED
> > TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +#
> > DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> > ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> > OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > OUT OF THE USE OF +# THIS SOFTWARE, EVEN IF ADVISED OF THE
> > POSSIBILITY OF SUCH DAMAGE. + +SUBDIRS = asinfo
> > diff --git a/tools/asinfo/Makefile.am b/tools/asinfo/Makefile.am
> > new file mode 100644
> > index 00000000..40d0df53
> > --- /dev/null
> > +++ b/tools/asinfo/Makefile.am
> > @@ -0,0 +1,57 @@
> > +# Automake input for strace.
> > +#
> > +# Copyright (c) 2017 Edgar Kaziakhmedov <edos at linux.com>
> > +# All rights reserved.
> > +#
> > +# Redistribution and use in source and binary forms, with or
> > without +# modification, are permitted provided that the following
> > conditions +# are met:
> > +# 1. Redistributions of source code must retain the above copyright
> > +#    notice, this list of conditions and the following disclaimer.
> > +# 2. Redistributions in binary form must reproduce the above
> > copyright +#    notice, this list of conditions and the following
> > disclaimer in the +#    documentation and/or other materials
> > provided with the distribution. +# 3. The name of the author may
> > not be used to endorse or promote products +#    derived from this
> > software without specific prior written permission. +#
> > +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR +# IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> > THE IMPLIED WARRANTIES +# OF MERCHANTABILITY AND FITNESS FOR A
> > PARTICULAR PURPOSE ARE DISCLAIMED. +# IN NO EVENT SHALL THE AUTHOR
> > BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL,
> > EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT +# NOT LIMITED
> > TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +#
> > DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> > ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> > OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> > OUT OF THE USE OF +# THIS SOFTWARE, EVEN IF ADVISED OF THE
> > POSSIBILITY OF SUCH DAMAGE. + +bin_PROGRAMS = asinfo
> > +man_MANS = asinfo.1
> > +
> > +OS = linux
> > +ARCH = @arch@
> > +
> > +AM_CFLAGS = $(WARN_CFLAGS)
> > +AM_CPPFLAGS = -I$(builddir) \
> > +	      -I$(top_builddir)/$(OS) \
> > +	      -I$(top_srcdir)/$(OS) \
> > +	      -I$(top_builddir) \
> > +	      -I$(top_srcdir)
> > +asinfo_CPPFLAGS = $(AM_CPPFLAGS)
> > +asinfo_CFLAGS = $(AM_CFLAGS)
> > +asinfo_LDADD = libasinfo.a
> > +noinst_LIBRARIES = libasinfo.a
> > +LIBS = -L${top_builddir} -lstrace  
> 
> The command "make distclean && ./bootstrap && ./configure && make"
> fails on my system with the following diagnostics:
> 
> 	ranlib libasinfo.a
> 	gcc -Wall -Wempty-body -Wformat-security -Wignored-qualifiers
> 	-Winit-self -Wlogical-op -Wmissing-parameter-type
> -Wnested-externs -Wold-style-declaration -Wold-style-definition
> -Wsign-compare -Wtype-limits -Wwrite-strings -g -O2   -o asinfo
> asinfo-asinfo.o libasinfo.a -L../.. -lstrace
> 	/usr/bin/ld: cannot find -lstrace
> 	collect2: error: ld returned 1 exit status
> 	Makefile:490: recipe for target 'asinfo' failed
> 
> I assume that libstrace is missing at the time of asinfo linking.
> 
> > +
> > +libasinfo_a_CPPFLAGS = $(asinfo_CPPFLAGS)
> > +libasinfo_a_CFLAGS = $(asinfo_CFLAGS)
> > +libasinfo_a_SOURCES =			\
> > +	dispatchers.c			\
> > +	dispatchers.h			\
> > +	xshortmalloc.c			\
> > +	#end of libasinfo_a_SOURCES
> > +
> > +asinfo_SOURCES = 	\  
> Tabs after spaces.
> 
> > +	asinfo.c	\
> > +	asinfo.h	\
> > +	#end of asinfo_SOURCES
> > diff --git a/tools/asinfo/arch_list.h b/tools/asinfo/arch_list.h
> > new file mode 100644
> > index 00000000..628d02e7
> > --- /dev/null
> > +++ b/tools/asinfo/arch_list.h
> > @@ -0,0 +1,5 @@
> > +[0] = { ARM64,		"arm64",
> > ARRAY_SIZE(aarch64_sysent0),
> > aarch64_sysent0		}, +[1] =
> > { AARCH64,	"aarch64",
> > ARRAY_SIZE(aarch64_sysent0),
> > aarch64_sysent0		},  
> What's the difference between these two? BTW, for i386, there at least
> four possible variants (i386/i486/i586/i686).
> 
> > +[2] = { ARM,		"arm",
> > ARRAY_SIZE(arm_sysent0),	arm_sysent0		}, +[3]
> > = { x86_64,		"x86_64",
> > ARRAY_SIZE(x86_64_sysent0),	x86_64_sysent0		},
> > +[4] = { x32,		"x32",
> > ARRAY_SIZE(x32_sysent0),	x32_sysent0		},  
> I'm not quite sure whether the explicit providing of array indices
> is needed here.
I am going to add generating this table after implementation main
functionality, that is why I moved this array into separate file. Sure,
manual generating is not convenient.
> 
> > diff --git a/tools/asinfo/asinfo.1 b/tools/asinfo/asinfo.1
> > new file mode 100644
> > index 00000000..e69de29b
> > diff --git a/tools/asinfo/asinfo.c b/tools/asinfo/asinfo.c
> > new file mode 100644
> > index 00000000..e82e954b
> > --- /dev/null
> > +++ b/tools/asinfo/asinfo.c
> > @@ -0,0 +1,96 @@
> > +/*
> > + * Copyright (c) 2017 Edgar A. Kaziakhmedov <edos at linux.com>
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > without
> > + * modification, are permitted provided that the following
> > conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer
> > in the
> > + *    documentation and/or other materials provided with the
> > distribution.
> > + * 3. The name of the author may not be used to endorse or promote
> > products
> > + *    derived from this software without specific prior written
> > permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR
> > + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> > WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> > DISCLAIMED.
> > + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> > (INCLUDING, BUT
> > + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS OF USE,
> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> > ON ANY
> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> > THE USE OF
> > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > + */
> > +
> > +#include "asinfo.h"  
> I'm not quite sure whether including local headers before the system
> ones is a good ides.
Why? I made it, because this header consists of general code for
library and asinfo tool.
> 
> > +#include <stdarg.h>
> > +#include <string.h>
> > +
> > +#include "dispatchers.h"
> > +#include "request_msgs.h"
> > +
> > +static const char *progname;
> > +
> > +static unsigned ATTRIBUTE_NOINLINE  
> Why do you need ATTRIBUTE_NOINLINE here?

> > +form_req(int argc, char *argv[])
> > +{
> > +	progname = argv[0] ? argv[0] : "asinfo";
> > +	return AD_REQ_LIST_ARCH;
> > +}
> > +
> > +int
> > +main(int argc, char* argv[]) {  
> Opening curly bracket should be on a separate line.
> 
> > +	unsigned requests = form_req(argc, argv);
> > +	ARCH_LIST_DEFINE(arch_dispatcher(requests, "NULL"));
> > +	int size;
> > +	ARCH_LIST_SIZE(size);
> > +	int i;
> > +	for (i = 0; i < size; i++)   
> Trailing whitespace.
> 
> > +		fprintf(stderr, "%10s : %3u\n",
> > ARCH_LIST_GET_NAME(i),
> > +
> > ARCH_LIST_GET_SNUM(i));   
> Trailing whitespace.
> 
> > +	ARCH_LIST_FREE;
> > +	return 0;
> > +}
> > +
> > +static void verror_msg(int err_no, const char *fmt, va_list p)
> > +{
> > +	char *msg;
> > +
> > +	fflush(NULL);
> > +
> > +	/* We want to print entire message with single fprintf to
> > ensure
> > +	 * message integrity if stderr is shared with other
> > programs.
> > +	 * Thus we use vasprintf + single fprintf.
> > +	 */
> > +	msg = NULL;
> > +	if (vasprintf(&msg, fmt, p) >= 0) {
> > +		if (err_no)
> > +			fprintf(stderr, "%s: %s: %s\n", progname,
> > msg,
> > +				strerror(err_no));
> > +		else
> > +			fprintf(stderr, "%s: %s\n", progname, msg);
> > +		free(msg);
> > +	} else {
> > +		/* malloc in vasprintf failed, try it without
> > malloc */
> > +		fprintf(stderr, "%s: ", progname);
> > +		vfprintf(stderr, fmt, p);
> > +		if (err_no)
> > +			fprintf(stderr, ": %s\n",
> > strerror(err_no));
> > +		else
> > +			putc('\n', stderr);
> > +	}
> > +	/* We don't switch stderr to buffered, thus fprintf(stderr)
> > +	 * always flushes its output and this is not necessary: */
> > +	/* fflush(stderr); */
> > +}
> > +
> > +void error_msg_and_die(const char *fmt, ...)
> > +{
> > +	va_list p;
> > +	va_start(p, fmt);
> > +	verror_msg(0, fmt, p);
> > +	exit(1);
> > +}  
> I'd avoid the duplication of these two function by refactoring them
> somewhere accessible for both asinfo and strace.
Actually, there is a small problem, because error_msg_and_die uses die()
function, which links to cleanup() function, that is completely useless
for asinfo.

> 
> > diff --git a/tools/asinfo/asinfo.h b/tools/asinfo/asinfo.h
> > new file mode 100644
> > index 00000000..fc6ef747
> > --- /dev/null
> > +++ b/tools/asinfo/asinfo.h
> > @@ -0,0 +1,95 @@
> > +/*
> > + * Copyright (c) 2017 Edgar A. Kaziakhmedov <edos at linux.com>
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > without
> > + * modification, are permitted provided that the following
> > conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer
> > in the
> > + *    documentation and/or other materials provided with the
> > distribution.
> > + * 3. The name of the author may not be used to endorse or promote
> > products
> > + *    derived from this software without specific prior written
> > permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR
> > + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> > WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> > DISCLAIMED.
> > + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> > (INCLUDING, BUT
> > + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS OF USE,
> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> > ON ANY
> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> > THE USE OF
> > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > + */
> > +
> > +#ifndef STRACE_ASINFO_H
> > +#define STRACE_ASINFO_H
> > +
> > +#ifdef HAVE_CONFIG_H
> > +# include "config.h"
> > +#endif
> > +
> > +#include <sys/types.h>
> > +#include <sys/utsname.h>
> > +#include <stdbool.h>
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +
> > +#include "kernel_types.h"
> > +#include "gcc_compat.h"
> > +
> > +
> > +
> > +#ifndef offsetofend
> > +# define offsetofend(type, member) \
> > +	(offsetof(type, member) + sizeof(((type *)NULL)->member))
> > +#endif
> > +
> > +#define ARRAY_SIZE(arg) ((unsigned int) (sizeof(arg) /
> > sizeof((arg)[0]))) +#define LENGTH_OF(arg) ((unsigned int)
> > sizeof(arg) - 1) +
> > +/*
> > + * The kernel used to define 64-bit types on 64-bit systems on a
> > per-arch
> > + * basis.  Some architectures would use unsigned long and others
> > would use
> > + * unsigned long long.  These types were exported as part of the
> > + * kernel-userspace ABI and now must be maintained forever.  This
> > matches
> > + * what the kernel exports for each architecture so we don't need
> > to cast
> > + * every printing of __u64 or __s64 to stdint types.
> > + */
> > +#if SIZEOF_LONG == 4
> > +# define PRI__64 "ll"
> > +#elif defined ALPHA || defined IA64 || defined MIPS || defined
> > POWERPC +# define PRI__64 "l"
> > +#else
> > +# define PRI__64 "ll"
> > +#endif
> > +
> > +#define PRI__d64 PRI__64"d"
> > +#define PRI__u64 PRI__64"u"
> > +#define PRI__x64 PRI__64"x"
> > +
> > +#if WORDS_BIGENDIAN
> > +# define LL_PAIR(HI, LO) (HI), (LO)
> > +#else
> > +# define LL_PAIR(HI, LO) (LO), (HI)
> > +#endif
> > +#define LL_VAL_TO_PAIR(llval) LL_PAIR((long) ((llval) >> 32),
> > (long) (llval)) +
> > +#define _STR(_arg) #_arg
> > +#define ARG_STR(_arg) (_arg), #_arg
> > +#define ARG_ULL_STR(_arg) _arg##ULL, #_arg
> > +
> > +void error_msg_and_die(const char *fmt, ...)
> > +	ATTRIBUTE_FORMAT((printf, 1, 2)) ATTRIBUTE_NORETURN;
> > +void die_out_of_memory(void) ATTRIBUTE_NORETURN;
> > +
> > +void *xmalloc(size_t size) ATTRIBUTE_MALLOC
> > ATTRIBUTE_ALLOC_SIZE((1)); +void *xcalloc(size_t nmemb, size_t size)
> > +	ATTRIBUTE_MALLOC ATTRIBUTE_ALLOC_SIZE((1, 2));  
> Again, I see no reason duplicating it here, as asinfo does not try to
> be isolated from strace tree anyway. Copyright claim is also bogus in
> this case, btw.
In this case, the solution is to create common.h header, which will
include these subroutines and their prototypes and form them as a
separate patch, ok, I will make it.
> 
> > +
> > +#endif /* !STRACE_ASINFO_H */
> > diff --git a/tools/asinfo/dispatchers.c b/tools/asinfo/dispatchers.c
> > new file mode 100644
> > index 00000000..bca16d21
> > --- /dev/null
> > +++ b/tools/asinfo/dispatchers.c
> > @@ -0,0 +1,159 @@
> > +/*
> > + * Copyright (c) 2017 Edgar A. Kaziakhmedov <edos at linux.com>
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > without
> > + * modification, are permitted provided that the following
> > conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer
> > in the
> > + *    documentation and/or other materials provided with the
> > distribution.
> > + * 3. The name of the author may not be used to endorse or promote
> > products
> > + *    derived from this software without specific prior written
> > permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR
> > + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> > WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> > DISCLAIMED.
> > + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> > (INCLUDING, BUT
> > + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS OF USE,
> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> > ON ANY
> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> > THE USE OF
> > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <string.h>
> > +#include <sys/utsname.h>
> > +
> > +#include "asinfo.h"
> > +#include "dispatchers.h"
> > +#include "request_msgs.h"
> > +#include "sysent.h"
> > +
> > +/* Define these shorthand notations to simplify the syscallent
> > files. */ +/* These flags could be useful to print group of
> > syscalls */ +#define TD TRACE_DESC
> > +#define TF TRACE_FILE
> > +#define TI TRACE_IPC
> > +#define TN TRACE_NETWORK
> > +#define TP TRACE_PROCESS
> > +#define TS TRACE_SIGNAL
> > +#define TM TRACE_MEMORY
> > +#define TST TRACE_STAT
> > +#define TLST TRACE_LSTAT
> > +#define TFST TRACE_FSTAT
> > +#define TSTA TRACE_STAT_LIKE
> > +#define TSF TRACE_STATFS
> > +#define TFSF TRACE_FSTATFS
> > +#define TSFA TRACE_STATFS_LIKE
> > +#define NF SYSCALL_NEVER_FAILS
> > +#define MA MAX_ARGS
> > +#define SI STACKTRACE_INVALIDATE_CACHE
> > +#define SE STACKTRACE_CAPTURE_ON_ENTER
> > +#define CST COMPAT_SYSCALL_TYPES
> > +
> > +/*
> > + * For the current functionality there is no need
> > + * to use sen and (*sys_func)() fields in sysent struct
> > + */
> > +#define SEN(syscall_name) 0, NULL
> > +
> > +struct_sysent aarch64_sysent0[] = {
> > +#include "aarch64/syscallent.h"
> > +};
> > +
> > +struct_sysent arm_sysent0[] = {
> > +#include "arm/syscallent.h"
> > +};
> > +
> > +struct_sysent x32_sysent0[] = {
> > +#include "x32/syscallent.h"
> > +};
> > +
> > +struct_sysent x86_64_sysent0[] = {
> > +#include "x86_64/syscallent.h"
> > +};
> > +
> > +#undef SEN
> > +#undef TD
> > +#undef TF
> > +#undef TI
> > +#undef TN
> > +#undef TP
> > +#undef TS
> > +#undef TM
> > +#undef TST
> > +#undef TLST
> > +#undef TFST
> > +#undef TSTA
> > +#undef TSF
> > +#undef TFSF
> > +#undef TSFA
> > +#undef NF
> > +#undef MA
> > +#undef SI
> > +#undef SE
> > +#undef CST
> > +
> > +struct arch_descriptor architectures[] = {
> > +#include "arch_list.h"
> > +};
> > +
> > +void   
> Trailing whitespace.
> 
> > +*syscall_dispatcher(char *arch, int request_type, void *data)
> > +{
> > +	//  
> Single-line comment.
> 
> > +	return NULL;
> > +}
> > +
> > +struct arch_descriptor   
> Trailing whitespace.
> 
> > +**arch_dispatcher(unsigned request_type, const char* arch)  
> Why the asterisks of the return type are here?
Oh, yes.
> 
> > +{
> > +	struct utsname info_uname;
> > +	const char* loc_arch;  
> const char *loc_arch;
> 
> > +	unsigned i;
> > +	ARCH_LIST_DEFINE(NULL);
> > +	if ((!arch) && (request_type & AD_REQ_CHECK_ARCH)) {
> > +		goto fail;
> > +	}
> > +
> > +	if (request_type & AD_REQ_GET_ARCH) {
> > +		uname(&info_uname);
> > +		for (i = 0; i < ARRAY_SIZE(architectures); i++) {
> > +			loc_arch = architectures[i].arch_name;
> > +			if (strcasestr(info_uname.machine,
> > loc_arch) != NULL) {  
> Why strcasestr?
for example, if somebody types AArch64 instead of
aarch64 it doesn't mean that he has mistaken, doesn't it?

> 
> > +				ARCH_LIST_ALLOC(1);
> > +				ARCH_LIST_ADD(0,
> > &(architectures[i]));
> > +				goto done;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (request_type & AD_REQ_CHECK_ARCH) {
> > +		for (i = 0; i < ARRAY_SIZE(architectures); i++) {
> > +			loc_arch = architectures[i].arch_name;
> > +			if (strcasestr(arch, loc_arch) != NULL) {
> > +				ARCH_LIST_ALLOC(1);
> > +				ARCH_LIST_ADD(0,
> > &(architectures[i]));
> > +				goto done;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (request_type & AD_REQ_LIST_ARCH) {
> > +		ARCH_LIST_ALLOC(ARRAY_SIZE(architectures));
> > +		for (i = 0; i < ARRAY_SIZE(architectures); i++) {
> > +			ARCH_LIST_ADD(i, &(architectures[i]));
> > +		}
> > +		goto done;
> > +	}  
> As of now, it looks like case switch.
> 
> > +
> > +fail:
> > +	ARCH_LIST_ALLOC(0);
> > +done:
> > +	return GET_ARCH_LIST;
> > +}
> > diff --git a/tools/asinfo/dispatchers.h b/tools/asinfo/dispatchers.h
> > new file mode 100644
> > index 00000000..bff225bc
> > --- /dev/null
> > +++ b/tools/asinfo/dispatchers.h
> > @@ -0,0 +1,99 @@
> > +/*
> > + * Copyright (c) 2017 Edgar A. Kaziakhmedov <edos at linux.com>
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > without
> > + * modification, are permitted provided that the following
> > conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer
> > in the
> > + *    documentation and/or other materials provided with the
> > distribution.
> > + * 3. The name of the author may not be used to endorse or promote
> > products
> > + *    derived from this software without specific prior written
> > permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR
> > + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> > WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> > DISCLAIMED.
> > + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> > (INCLUDING, BUT
> > + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS OF USE,
> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> > ON ANY
> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> > THE USE OF
> > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > + */
> > +
> > +#ifndef STRACE_ASINFO_COM_SCALLENT
> > +#define STRACE_ASINFO_COM_SCALLENT
> > +
> > +#include "sysent.h"
> > +#include "asinfo.h"
> > +
> > +enum arch_types {
> > +	NO_ARCH,
> > +	ARM64,
> > +	AARCH64,
> > +	ARM,
> > +	x86_64,
> > +	x32
> > +};
> > +
> > +struct arch_descriptor {
> > +	enum arch_types arch_num;
> > +	const char *arch_name;
> > +	int max_scn;
> > +	struct_sysent *list_of_syscall;
> > +};
> > +
> > +#define MAX_ARCH_LEN 10  
> Why 10?
Just test constant, after that this constant will be calculated
based on current archs.
> 
> > +
> > +/* These defines are such useful to work with array of
> > + * struct arch_descriptor pointers  
> I'm not very sure that using macros here is a good idea. Too much for
> code obfuscation.
Because working with array of pointers to pointers is quite comfortable.
> 
> > + */
> > +#define
> > ARCH_LIST_DEFINE(init_val)					 \
> > +	struct arch_descriptor **arch_list =
> > (init_val)			 \ +
> > +#define ARCH_LIST_ALLOC(size) do
> > {					 \
> > +	arch_list = (struct
> > arch_descriptor**)xcalloc(sizeof(*arch_list),\  
> (struct arch_descriptor **) xcalloc(sizeof(*arch_list),
> 
> > +						      (size +
> > 1));	 \
> > +	arch_list[size] =
> > NULL;						 \ +}
> > while(0)
> > \  
> Missing space after while. Macro body indentation is also missing.
Hm , it is strange, it happened because I decided to frame define with
tabs, but in the patch it doen't look the same. Fixed.
> 
> > +
> > +#define ARCH_LIST_ADD(pos, elem) do
> > {					 \
> > +	arch_list[(pos)] =
> > elem;					 \ +}
> > while(0)
> > \  
> Missing space after while. Macro body indentation is also missing.
> 
> > +
> > +#define ARCH_LIST_FREE do
> > {						 \
> > +
> > free(arch_list);						 \
> > +}
> > while(0)
> > \  
> Missing space after while. Macro body indentation is also missing.
> 
> > +
> > +/* In the future, it will be possible to filter architectures, thus
> > + * size could less than max number of supported archs */  
> Since arch list is opaque, it could also store information about its
> actual size and capacity and implement some push_back interface.
Good tip.
> 
> > +#define ARCH_LIST_SIZE(size) do
> > {					 \
> > +	int i =
> > 0;							 \
> > +	while (arch_list[i] != NULL)
> > i++;				 \
> > +	(size) =
> > i;							 \ +}
> > while(0)
> > \ + +#define ARCH_LIST_GET_NAME(pos)
> > arch_list[(pos)] ?			 \
> > +				arch_list[(pos)]->arch_name :
> > "NULL"	 \ +
> > +#define ARCH_LIST_GET_SNUM(pos)
> > arch_list[(pos)] ?			 \
> > +				arch_list[(pos)]->max_scn :
> > 0		 \ +
> > +#define GET_ARCH_LIST
> > arch_list						 \ +
> > +/* The main interaction with low-level structures, such as for now,
> > + * struct_sysent, is happening here, the remaining processing
> > should
> > + * be done on the other levels.
> > + */
> > +void *syscall_dispatcher(char *arch, int request_type, void *data);
> > +
> > +/* The function is purposed to interact with uname syscall
> > + */
> > +struct arch_descriptor **arch_dispatcher(unsigned request_type,
> > +					 const char* arch);
> > +
> > +#endif /* !STRACE_ASINFO_COM_SCALLENT */
> > diff --git a/tools/asinfo/request_msgs.h
> > b/tools/asinfo/request_msgs.h new file mode 100644
> > index 00000000..73240dcb
> > --- /dev/null
> > +++ b/tools/asinfo/request_msgs.h
> > @@ -0,0 +1,72 @@
> > +/*
> > + * Copyright (c) 2017 Edgar A. Kaziakhmedov <edos at linux.com>
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > without
> > + * modification, are permitted provided that the following
> > conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer
> > in the
> > + *    documentation and/or other materials provided with the
> > distribution.
> > + * 3. The name of the author may not be used to endorse or promote
> > products
> > + *    derived from this software without specific prior written
> > permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR
> > + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> > WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> > DISCLAIMED.
> > + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> > (INCLUDING, BUT
> > + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS OF USE,
> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> > ON ANY
> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> > THE USE OF
> > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > + */
> > +
> > +#ifndef STRACE_ASINFO_REQ_MSGS
> > +#define STRACE_ASINFO_REQ_MSGS
> > +
> > +
> > +/* Request types for syscall_dispatcher,
> > + * arch_dispatcher, and filter_dispatcher,
> > + * which, in turn, could be combined
> > + */
> > +enum {
> > +	SD_REQ_GET_NAME		= 0x1LU,
> > +	SD_REQ_GET_NUMBER	= 0x2LU,
> > +	SD_REQ_GET_NARGS	= 0x4LU,
> > +	SD_REQ_GET_PROTO	= 0x8LU,
> > +	SD_REQ_GET_KERNEL	= 0x10LU,
> > +	SD_REQ_GET_LIST		= 0x20LU,
> > +	/* Reserved */
> > +	AD_REQ_SET_ARCH		= 0x1LU<<16,
> > +	AD_REQ_GET_ARCH		= 0x2LU<<16,
> > +	AD_REQ_LIST_ARCH	= 0x4LU<<16,
> > +	AD_REQ_CHECK_ARCH	= 0x8LU<<16,
> > +	/* Reserved */
> > +	FD_SET_BASE_FILTER	= 0x1LU<<24,
> > +	FD_SET_ADV_FILTER	= 0x2LU<<24  
> Enumeration values are integers, no need to use long unsigned integral
> constants in order to define them. Also, missing spaces around shift
> left operator.
Since I sent this patch, I have fixed it. 
> 
> > +};
> > +
> > +/* SD_REQ_GET_NAME, SD_REQ_GET_NUMBER */
> > +struct sc_base_info {
> > +	int sys_number;
> > +	char *sys_name;
> > +	int sys_flags;
> > +};
> > +
> > +/* SD_REQ_GET_NARGS */
> > +struct sc_nargs {
> > +	struct sc_base_info sys_bi;
> > +	int nargs;
> > +};
> > +
> > +/* FD_SET_BASE_FILTER */
> > +struct sc_filter {
> > +	struct sc_base_info sys_bi;
> > +	int filter_options;
> > +};
> > +
> > +#endif /* !STRACE_ASINFO_REQ_MSGS */
> > diff --git a/tools/asinfo/xshortmalloc.c
> > b/tools/asinfo/xshortmalloc.c new file mode 100644
> > index 00000000..1ad9768f
> > --- /dev/null
> > +++ b/tools/asinfo/xshortmalloc.c
> > @@ -0,0 +1,59 @@
> > +/*
> > + * Copyright (c) 2015 Dmitry V. Levin <ldv at altlinux.org>
> > + * All rights reserved.
> > + *
> > + * Redistribution and use in source and binary forms, with or
> > without
> > + * modification, are permitted provided that the following
> > conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above
> > copyright
> > + *    notice, this list of conditions and the following disclaimer
> > in the
> > + *    documentation and/or other materials provided with the
> > distribution.
> > + * 3. The name of the author may not be used to endorse or promote
> > products
> > + *    derived from this software without specific prior written
> > permission.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY
> > EXPRESS OR
> > + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> > WARRANTIES
> > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> > DISCLAIMED.
> > + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> > + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> > (INCLUDING, BUT
> > + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS OF USE,
> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> > ON ANY
> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> > THE USE OF
> > + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > + */
> > +
> > +#include "asinfo.h"
> > +
> > +void die_out_of_memory(void)
> > +{
> > +	static bool recursed = false;
> > +
> > +	if (recursed)
> > +		exit(1);
> > +	recursed = 1;
> > +
> > +	error_msg_and_die("Out of memory");
> > +}
> > +
> > +void *xmalloc(size_t size)
> > +{
> > +	void *p = malloc(size);
> > +
> > +	if (!p)
> > +		die_out_of_memory();
> > +
> > +	return p;
> > +}
> > +
> > +void *xcalloc(size_t nmemb, size_t size)
> > +{
> > +	void *p = calloc(nmemb, size);
> > +
> > +	if (!p)
> > +		die_out_of_memory();
> > +
> > +	return p;
> > +}  
> Again, see no reasons for code duplication.
> 
> Can't quite grasp the need of using "NULL" as a dummy architecture.
With push_back interface, there is no need, but with the current
implementation, it's sort of "end of string".
> 
> Otherwise, I'm looking forward to the next iteration of the patch
> where more commands will be implemented. I assume some command
> dispatcher should be added to main in order to realize that.
> 
> ------------------------------------------------------------------------------
> 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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170620/626e2cc9/attachment.bin>


More information about the Strace-devel mailing list