On Mon, 19 Jun 2017 08:30:26 +0200 Eugene Syromiatnikov <e...@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 <e...@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 <e...@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 <e...@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 <e...@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 <e...@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 <e...@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 <e...@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 <e...@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 <l...@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@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel
pgpPBUCVWcLSj.pgp
Description: OpenPGP digital signature
------------------------------------------------------------------------------ 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
