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
signature.asc
Description: PGP 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
