[PATCH 1/2] Replace errs and mem subroutines to separate files
Eugene Syromiatnikov
esyr at redhat.com
Mon Jul 10 05:15:45 UTC 2017
On Wed, Jun 28, 2017 at 05:56:39AM +0300, Edgar Kaziahmedov wrote:
> According to the current state of affairs, strace.c contains
"According to the current state of affairs" makes little sense to me. It
probably will be better if the reason for the change is mentioned in the
first place, like "In order to enable/allow usage/leveraging of utility
functions and macros by other binaries included in the strace package
(like the upcoming asinfo utility), these function should be moved..."
> functions, which could be replaced to separate file, for instance,
"functions which should be moved/put" (no comma is needed)
> file with functions printing errors. Also it could be useful for
"file with error printing functions"
> the future, because other binaries, included to the strace package,
> will use wrappers for printing errors, thus it would avoid the
"other binaries included in the strace package will use" (no commas are needed)
> duplication of code. Now, all common prototypes are placed in common.h.
I do not see any trace of common.h presence.
> NOTE: asinfo also needs these subroutines.
>
> * Makefile.am (strace_SOURCES): Add xmalloc.h, macros.h, error_prints.h,
> and error_prints.c.
It's better to have this list sorted.
> * macros.h: New file.
> (ARRAY_SIZE): Move from defs.h.
> * error_prints.h: New file.
> (error_msg, perror_msg, perror_msg_and_die, error_msg_and_help,
> error_msg_and_die): Move from defs.h.
> * xmalloc.h: New file.
> (xcalloc, xmalloc, xreallocarray, xstrdup, xstrndup): Move from defs.h.
> * defs.h: Include "xmalloc.h", "macros.h", and "error_prints.h".
> (error_msg, perror_msg, perror_msg_and_die, error_msg_and_help,
> error_msg_and_die, xcalloc, xmalloc, xreallocarray, xstrdup, xstrndup,
> ARRAY_SIZE): Move to corresponding header.
> * error_prints.c (verror_msg, error_msg, perror_msg, perror_msg_and_die,
> error_msg_and_help, error_msg_and_die, die_out_of_memory): Move from
> strace.c.
> * strace.c (verror_msg, error_msg, perror_msg, perror_msg_and_die,
> error_msg_and_help, error_msg_and_die, die_out_of_memory): Move to
> error_prints.c.
The idiom in this case is like this:
* strace.c (verror_msg, error_msg, perror_msg, perror_msg_and_die,
error_msg_and_help, error_msg_and_die, die_out_of_memory): Move...
* error_prints.c: ... to the new file.
> (progname): Remove static qualifier to make visible for error_prints.c.
> (die): Likewise.
> * xmalloc.c: Remove "defs.h". Add "xmalloc.h".
>
> Signed-off-by: Edgar Kaziahmedov <edos at frtk.ru>
> ---
> Makefile.am | 4 ++
> defs.h | 22 ++---------
> error_prints.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> error_prints.h | 42 ++++++++++++++++++++
> macros.h | 35 +++++++++++++++++
> strace.c | 78 +-------------------------------------
> xmalloc.c | 8 +++-
> xmalloc.h | 41 ++++++++++++++++++++
> 8 files changed, 252 insertions(+), 96 deletions(-)
> create mode 100644 error_prints.c
> create mode 100644 error_prints.h
> create mode 100644 macros.h
> create mode 100644 xmalloc.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 76be3ca3..233796b6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -107,6 +107,8 @@ strace_SOURCES = \
> dyxlat.c \
> empty.h \
> epoll.c \
> + error_prints.c \
> + error_prints.h \
> evdev.c \
> eventfd.c \
> execve.c \
> @@ -159,6 +161,7 @@ strace_SOURCES = \
> lookup_dcookie.c \
> loop.c \
> lseek.c \
> + macros.h \
> mem.c \
> membarrier.c \
> memfd_create.c \
> @@ -273,6 +276,7 @@ strace_SOURCES = \
> xlat.c \
> xlat.h \
> xmalloc.c \
> + xmalloc.h \
> # end of strace_SOURCES
>
> if USE_LIBUNWIND
> diff --git a/defs.h b/defs.h
> index dcb4d947..86d3f47a 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -54,6 +54,9 @@
> #include <sys/time.h>
>
> #include "kernel_types.h"
> +#include "xmalloc.h"
It's better to keep list of include files sorted.
> +#include "error_prints.h"
> +#include "macros.h"
> #include "mpers_type.h"
> #include "gcc_compat.h"
> #include "sysent.h"
> @@ -75,8 +78,6 @@ extern char *stpcpy(char *dst, const char *src);
> (offsetof(type, member) + sizeof(((type *)NULL)->member))
> #endif
>
> -#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]) + MUST_BE_ARRAY(a))
> -
> /* macros */
> #ifndef MAX
> # define MAX(a, b) (((a) > (b)) ? (a) : (b))
> @@ -385,23 +386,6 @@ extern unsigned os_release;
> #undef KERNEL_VERSION
> #define KERNEL_VERSION(a, b, c) (((a) << 16) + ((b) << 8) + (c))
>
> -void error_msg(const char *fmt, ...) ATTRIBUTE_FORMAT((printf, 1, 2));
> -void perror_msg(const char *fmt, ...) ATTRIBUTE_FORMAT((printf, 1, 2));
> -void error_msg_and_die(const char *fmt, ...)
> - ATTRIBUTE_FORMAT((printf, 1, 2)) ATTRIBUTE_NORETURN;
> -void error_msg_and_help(const char *fmt, ...)
> - ATTRIBUTE_FORMAT((printf, 1, 2)) ATTRIBUTE_NORETURN;
> -void perror_msg_and_die(const char *fmt, ...)
> - ATTRIBUTE_FORMAT((printf, 1, 2)) 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));
> -void *xreallocarray(void *ptr, size_t nmemb, size_t size)
> - ATTRIBUTE_ALLOC_SIZE((2, 3));
> -char *xstrdup(const char *str) ATTRIBUTE_MALLOC;
> -char *xstrndup(const char *str, size_t n) ATTRIBUTE_MALLOC;
> -
> extern int read_int_from_file(const char *, int *);
>
> extern void set_sortby(const char *);
> diff --git a/error_prints.c b/error_prints.c
> new file mode 100644
> index 00000000..9817ca7b
> --- /dev/null
> +++ b/error_prints.c
> @@ -0,0 +1,118 @@
> +/*
File description will not harm here.
> + * Copyright (c) 1999-2017 The strace developers.
> + * 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.
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +
> +#include <errno.h>
> +#include <stdarg.h>
> +#include "string.h"
> +#include "stdio.h"
> +#include "stdlib.h"
> +
> +#include "error_prints.h"
> +
> +/* progname has to be defined globally */
> +extern const char* progname;
> +/* die() has to be implemented in the file, which
> + * is going to include this source */
> +extern void ATTRIBUTE_NORETURN die(void);
> +
> +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(const char *fmt, ...)
> +{
> + va_list p;
> + va_start(p, fmt);
> + verror_msg(0, fmt, p);
> + va_end(p);
> +}
> +
> +void error_msg_and_die(const char *fmt, ...)
> +{
> + va_list p;
> + va_start(p, fmt);
> + verror_msg(0, fmt, p);
> + die();
> +}
> +
> +void error_msg_and_help(const char *fmt, ...)
> +{
> + if (fmt != NULL) {
> + va_list p;
> + va_start(p, fmt);
> + verror_msg(0, fmt, p);
> + }
> + fprintf(stderr, "Try '%s -h' for more information.\n", progname);
> + die();
> +}
> +
> +void perror_msg(const char *fmt, ...)
> +{
> + va_list p;
> + va_start(p, fmt);
> + verror_msg(errno, fmt, p);
> + va_end(p);
> +}
> +
> +void perror_msg_and_die(const char *fmt, ...)
> +{
> + va_list p;
> + va_start(p, fmt);
> + verror_msg(errno, fmt, p);
> + die();
> +}
> diff --git a/error_prints.h b/error_prints.h
> new file mode 100644
> index 00000000..a18b4a9f
> --- /dev/null
> +++ b/error_prints.h
> @@ -0,0 +1,42 @@
> +/*
File description will not harm here.
> + * Copyright (c) 2001-2017 The strace developers.
> + * 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_ERROR_PRINTS_H
> +#define STRACE_ERROR_PRINTS_H
> +
> +#include "gcc_compat.h"
> +
> +void error_msg(const char *fmt, ...) ATTRIBUTE_FORMAT((printf, 1, 2));
> +void perror_msg(const char *fmt, ...) ATTRIBUTE_FORMAT((printf, 1, 2));
> +void perror_msg_and_die(const char *fmt, ...)
> + ATTRIBUTE_FORMAT((printf, 1, 2)) ATTRIBUTE_NORETURN;
> +void error_msg_and_help(const char *fmt, ...)
> + ATTRIBUTE_FORMAT((printf, 1, 2)) ATTRIBUTE_NORETURN;
> +void error_msg_and_die(const char *fmt, ...)
> + ATTRIBUTE_FORMAT((printf, 1, 2)) ATTRIBUTE_NORETURN;
> +
> +#endif /* !STRACE_ERROR_PRINTS_H */
> diff --git a/macros.h b/macros.h
> new file mode 100644
> index 00000000..559b16fb
> --- /dev/null
> +++ b/macros.h
> @@ -0,0 +1,35 @@
> +/*
File description will not harm here.
> + * Copyright (c) 2001-2017 The strace developers.
> + * 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_MACROS_H
> +#define STRACE_MACROS_H
> +
> +#include "gcc_compat.h"
> +
> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]) + MUST_BE_ARRAY(a))
> +
> +#endif /* !STRACE_MACROS_H */
> diff --git a/strace.c b/strace.c
> index 2bb4ec15..dbebc1d7 100644
> --- a/strace.c
> +++ b/strace.c
> @@ -151,7 +151,7 @@ static struct tcb *current_tcp;
>
> static struct tcb **tcbtab;
> static unsigned int nprocs, tcbtabsize;
> -static const char *progname;
> +const char *progname;
>
> unsigned os_release; /* generated from uname()'s u.release */
>
> @@ -275,7 +275,7 @@ Miscellaneous:\n\
> exit(0);
> }
>
> -static void ATTRIBUTE_NORETURN
> +void ATTRIBUTE_NORETURN
> die(void)
> {
> if (strace_tracer_pid == getpid()) {
> @@ -285,80 +285,6 @@ die(void)
> exit(1);
> }
>
> -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(const char *fmt, ...)
> -{
> - va_list p;
> - va_start(p, fmt);
> - verror_msg(0, fmt, p);
> - va_end(p);
> -}
> -
> -void error_msg_and_die(const char *fmt, ...)
> -{
> - va_list p;
> - va_start(p, fmt);
> - verror_msg(0, fmt, p);
> - die();
> -}
> -
> -void error_msg_and_help(const char *fmt, ...)
> -{
> - if (fmt != NULL) {
> - va_list p;
> - va_start(p, fmt);
> - verror_msg(0, fmt, p);
> - }
> - fprintf(stderr, "Try '%s -h' for more information.\n", progname);
> - die();
> -}
> -
> -void perror_msg(const char *fmt, ...)
> -{
> - va_list p;
> - va_start(p, fmt);
> - verror_msg(errno, fmt, p);
> - va_end(p);
> -}
> -
> -void perror_msg_and_die(const char *fmt, ...)
> -{
> - va_list p;
> - va_start(p, fmt);
> - verror_msg(errno, fmt, p);
> - die();
> -}
> -
> static void
> error_opt_arg(int opt, const char *arg)
> {
> diff --git a/xmalloc.c b/xmalloc.c
> index 43e93eb3..4578eb93 100644
> --- a/xmalloc.c
> +++ b/xmalloc.c
> @@ -25,7 +25,13 @@
> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> -#include "defs.h"
> +#include "stdbool.h"
> +#include "stdlib.h"
> +#include "string.h"
> +#include "stdio.h"
These probably shouldn't be searched in local include directories.
> +
> +#include "error_prints.h"
> +#include "xmalloc.h"
>
> static void die_out_of_memory(void)
> {
> diff --git a/xmalloc.h b/xmalloc.h
> new file mode 100644
> index 00000000..03375e28
> --- /dev/null
> +++ b/xmalloc.h
> @@ -0,0 +1,41 @@
> +/*
File description will not harm here.
> + * Copyright (c) 2001-2017 The strace developers.
> + * 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_XMALLOC_H
> +#define STRACE_XMALLOC_H
> +
> +#include "gcc_compat.h"
> +
> +void *xcalloc(size_t nmemb, size_t size)
> + ATTRIBUTE_MALLOC ATTRIBUTE_ALLOC_SIZE((1, 2));
> +void *xmalloc(size_t size) ATTRIBUTE_MALLOC ATTRIBUTE_ALLOC_SIZE((1));
> +void *xreallocarray(void *ptr, size_t nmemb, size_t size)
> + ATTRIBUTE_ALLOC_SIZE((2, 3));
> +char *xstrdup(const char *str) ATTRIBUTE_MALLOC;
> +char *xstrndup(const char *str, size_t n) ATTRIBUTE_MALLOC;
> +
> +#endif /* !STRACE_XMALLOC_H */
> --
> 2.11.0
More information about the Strace-devel
mailing list