[PATCH v8 02/12] Introduce new filtering architecture
Dmitry V. Levin
ldv at altlinux.org
Tue Aug 22 17:33:01 UTC 2017
On Tue, Aug 22, 2017 at 01:01:43PM +0700, Nikolay Marchuk wrote:
> This change introduces new filtering architecture primitives: filter,
> filter_action and bool_expression. Filtering is now done after decoding of
> syscall and tcp->qual_flg stores filtering results.
>
> * basic_actions.c: New file.
> * basic_filters.c (number_isset): Return bool.
> (is_number_in_set): Likewise.
> (add_number_to_set): Make static.
> (qualify_syscall_number): Rename to parse_syscall_number.
> (qualify_syscall_regex): Rename to parse_syscall_regex.
> (qualify_syscall_class): Rename to parse_syscall_class.
> (qualify_syscall_name): Rename to parse_syscall_name.
> (qualify_syscall): Rename to parse_syscall, use renamed functions.
> (qualify_syscall_tokens): Rename to parse_syscall_set, make static,
> remove "name" argument (assume "system call"), remove 'handle_inversion'
> label, remove additional clearing of the sets.
> (qualify_tokens): Rename to parse_set, remove 'handle_inversion' label,
> remove additional clearing of the set.
> (parse_syscall_filter, parse_fd_filter, parse_path_filter,
> run_syscall_filter, run_fd_filter, run_path_filter,
> free_syscall_filter, free_fd_filter, free_path_filter): New functions.
> * defs.h (struct inject_opts): Add init flag.
> (QUAL_READ, QUAL_WRITE): Change description.
> (traced, dump_read, dump_write): Add macros for checking QUAL_TRACE,
> QUAL_READ, QUAL_WRITE.
> (read_set, write_set): Remove global set variables.
> (qualify, qual_flags): Remove old declarations ...
> (filter_syscall, parse_qualify_filter, filtering_parsing_finish):
> ... and add new declarations.
> * filter.c: New file.
> * filter.h: Change declarations.
> * filter_action.c: New file.
> * filter_expression.c: Likewise.
> * filter_qualify.c (read_set, write_set, abbrev_set, inject_set, raw_set,
> trace_set, verbose_set): Remove set variables.
> (parse_inject_expression): Remove function.
> (parse_inject_common_args): New function.
> (qualify_read): Rename to parse_read.
> (qualify_write): Rename to parse_write.
> (qualify_signals): Use parse_set function. Add clearing of the set.
> (qualify_trace): Rename to parse_trace.
> (qualify_abbrev): Rename to parse_abbrev.
> (qualify_verbose): Rename to parse_verbose.
> (qualify_raw): Rename to parse_raw.
> (qualify_inject_common): Rename to parse_inject_common_qualify, use
> new filters.
> (qualify_fault): Rename to parse_fault.
> (qualify_inject): Rename to parse_inject.
> (qualify): Rename to parse_qualify_filter.
> (qual_flags): Remove function.
> * Makefile.am (strace_SOURCES): Add new files.
> * pathtrace.c (storepath): Duplicate paths.
> * strace.c (init): Use new filtering for -e option.
> (trace_syscall): Add filtering after syscall decoding.
> (droptcb): Don't free inject_vec for empty personalities.
> * syscall.c (decode_socket_subcall): Remove qual_flags from decoder.
> (decode_ipc_subcall): Likewise.
> (decode_mips_subcall): Likewise.
> (get_scno): Likewise.
> (inject_vec, tamper_with_syscall_entering): Remove inject_vec support code.
> (syscall_entering_trace): Use traced macro
> (dumpio): Check new macros instead of global sets.
> ---
> Makefile.am | 4 +
> basic_actions.c | 130 ++++++++++++++++++++++++
> basic_filters.c | 169 ++++++++++++++++++++++---------
> defs.h | 15 +--
> filter.c | 144 +++++++++++++++++++++++++++
> filter.h | 37 +++++--
> filter_action.c | 240 ++++++++++++++++++++++++++++++++++++++++++++
> filter_expression.c | 281 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> filter_qualify.c | 242 ++++++++++++++++++++------------------------
> pathtrace.c | 2 +-
> strace.c | 18 ++--
> syscall.c | 24 +----
> xmalloc.c | 3 +
> 13 files changed, 1090 insertions(+), 219 deletions(-)
> create mode 100644 basic_actions.c
> create mode 100644 filter.c
> create mode 100644 filter_action.c
> create mode 100644 filter_expression.c
First of all, this patch is too large, much larger than I can review.
If you do pure renaming (btw, why do you want to rename all these
functions?), please do not mix it with other changes.
> diff --git a/basic_filters.c b/basic_filters.c
> index 1dbdeac8..c6b5497e 100644
> --- a/basic_filters.c
> +++ b/basic_filters.c
> @@ -48,7 +48,8 @@ number_setbit(const unsigned int i, number_slot_t *const vec)
> static bool
> number_isset(const unsigned int i, const number_slot_t *const vec)
> {
> - return vec[i / BITS_PER_SLOT] & ((number_slot_t) 1 << (i % BITS_PER_SLOT));
> + return !!((vec[i / BITS_PER_SLOT]
> + & ((number_slot_t) 1 << (i % BITS_PER_SLOT))));
I don't understand why do you need this ...
> }
>
> static void
> @@ -62,7 +63,7 @@ reallocate_number_set(struct number_set *const set, const unsigned int new_nslot
> set->nslots = new_nslots;
> }
>
> -void
> +static void
> add_number_to_set(const unsigned int number, struct number_set *const set)
> {
> reallocate_number_set(set, number / BITS_PER_SLOT + 1);
> @@ -72,12 +73,12 @@ add_number_to_set(const unsigned int number, struct number_set *const set)
> bool
> is_number_in_set(const unsigned int number, const struct number_set *const set)
> {
> - return ((number / BITS_PER_SLOT < set->nslots)
> - && number_isset(number, set->vec)) ^ set->not;
> + return !!(((number / BITS_PER_SLOT < set->nslots)
> + && number_isset(number, set->vec)) ^ set->not);
and this.
> }
>
> static bool
> -qualify_syscall_number(const char *s, struct number_set *set)
> +parse_syscall_number(const char *s, struct number_set *set)
> {
> int n = string_to_uint(s);
> if (n < 0)
> @@ -108,7 +109,7 @@ regerror_msg_and_die(int errcode, const regex_t *preg,
> }
>
> static bool
> -qualify_syscall_regex(const char *s, struct number_set *set)
> +parse_syscall_regex(const char *s, struct number_set *set)
> {
> regex_t preg;
> int rc;
> @@ -180,7 +181,7 @@ lookup_class(const char *s)
> }
>
> static bool
> -qualify_syscall_class(const char *s, struct number_set *set)
> +parse_syscall_class(const char *s, struct number_set *set)
> {
> const unsigned int n = lookup_class(s);
> if (!n)
> @@ -203,7 +204,7 @@ qualify_syscall_class(const char *s, struct number_set *set)
> }
>
> static bool
> -qualify_syscall_name(const char *s, struct number_set *set)
> +parse_syscall_name(const char *s, struct number_set *set)
> {
> unsigned int p;
> bool found = false;
> @@ -225,7 +226,7 @@ qualify_syscall_name(const char *s, struct number_set *set)
> }
>
> static bool
> -qualify_syscall(const char *token, struct number_set *set)
> +parse_syscall(const char *token, struct number_set *set)
> {
> bool ignore_fail = false;
>
> @@ -234,11 +235,11 @@ qualify_syscall(const char *token, struct number_set *set)
> ignore_fail = true;
> }
> if (*token >= '0' && *token <= '9')
> - return qualify_syscall_number(token, set) || ignore_fail;
> + return parse_syscall_number(token, set) || ignore_fail;
> if (*token == '/')
> - return qualify_syscall_regex(token + 1, set) || ignore_fail;
> - return qualify_syscall_class(token, set)
> - || qualify_syscall_name(token, set)
> + return parse_syscall_regex(token + 1, set) || ignore_fail;
> + return parse_syscall_class(token, set)
> + || parse_syscall_name(token, set)
> || ignore_fail;
> }
>
These changes look like pure renames. Please provide a justification
for this change and please do not mix renames with other changes.
> diff --git a/filter_qualify.c b/filter_qualify.c
> index 4283e769..ff54720a 100644
> --- a/filter_qualify.c
> +++ b/filter_qualify.c
> @@ -38,16 +38,8 @@ struct number_set {
> bool not;
> };
>
> -struct number_set read_set;
> -struct number_set write_set;
> struct number_set signal_set;
>
> -static struct number_set abbrev_set[SUPPORTED_PERSONALITIES];
> -static struct number_set inject_set[SUPPORTED_PERSONALITIES];
> -static struct number_set raw_set[SUPPORTED_PERSONALITIES];
> -static struct number_set trace_set[SUPPORTED_PERSONALITIES];
> -static struct number_set verbose_set[SUPPORTED_PERSONALITIES];
Note that both basic_filters.c and filter_qualify.c define the same
structure number_set. This is absolutely unacceptable and I'm sorry
I missed it while reviewing b75c5b14, otherwise it wouldn't appear
in the first place.
I think I'll move all these number_set functions to a separate file
and create a proper API header.
> + /* Clear qual_flg to differ valid syscall from printargs */
s/differ/distinguish/
> diff --git a/xmalloc.c b/xmalloc.c
> index 45ff57b1..b77f6910 100644
> --- a/xmalloc.c
> +++ b/xmalloc.c
> @@ -91,6 +91,9 @@ xreallocarray(void *ptr, size_t nmemb, size_t size)
> char *
> xstrdup(const char *str)
> {
> + if (!str)
> + return NULL;
> +
> char *p = strdup(str);
>
> if (!p)
Please do not mix this kind of changes with other stuff.
--
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170822/6e801064/attachment.bin>
More information about the Strace-devel
mailing list