[PATCH v3 3/6] Implement parsing of inject and fault filter actions
Eugene Syromiatnikov
esyr at redhat.com
Mon Jul 3 12:02:45 UTC 2017
On Thu, Jun 29, 2017 at 02:46:12PM +0700, Nikolay Marchuk wrote:
> * basic_actions.c (parse_inject, parse_fault, apply_fault): Implement.
> (apply_inject): Change inject private data.
> * defs.h (inject_vec): Remove.
> (inject_opts): Add init flag.
> * filter.h (parse_inject_token): Add definition.
> * filter_action.c: Add fault action.
> * filter_qualify.c (parse_inject_common): Generate new inject private data.
> ---
> basic_actions.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> defs.h | 2 +-
> filter.h | 2 ++
> filter_action.c | 3 ++
> filter_qualify.c | 42 ++++++++++---------------
> 5 files changed, 108 insertions(+), 36 deletions(-)
>
> diff --git a/basic_actions.c b/basic_actions.c
> index de87cde..1eb597e 100644
> --- a/basic_actions.c
> +++ b/basic_actions.c
> @@ -25,6 +25,7 @@
> * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
> #include "defs.h"
> +#include "filter.h"
>
> bool
> is_traced(struct tcb *tcp)
> @@ -49,50 +50,134 @@ apply_trace(struct tcb *tcp, void *_priv_data)
> {
> tcp->qual_flg |= QUAL_TRACE;
> }
> +
> +struct inject_priv_data {
> + struct inject_opts opts;
> + struct inject_opts **inject_vec;
> +};
> +
> void
> apply_inject(struct tcb *tcp, void *_priv_data)
> {
> - struct inject_opts **inject_vec = (struct inject_opts **)_priv_data;
> + struct inject_priv_data *data = (struct inject_priv_data *)_priv_data;
> + struct inject_opts *opts;
> + if (!(scno_in_range(tcp->scno) && data->inject_vec[current_personality]))
Overlength line.
> + return;
> + opts = &data->inject_vec[current_personality][tcp->scno];
> + if (!opts->init) {
> + data->inject_vec[current_personality][tcp->scno] = data->opts;
> + opts = &data->inject_vec[current_personality][tcp->scno];
> + opts->init = true;
> + }
> tcp->qual_flg |= QUAL_INJECT;
> - tcp->inject_opts = (scno_in_range(tcp->scno) && inject_vec[current_personality])
Overlength line.
> - ? &inject_vec[current_personality][tcp->scno] : NULL;
> + tcp->inject_opts = opts;
> }
> -void*
> +
> +static void *
> +parse_inject_common_args(const char *const str,
> + const bool fault_tokens_only,
> + const char *const description)
> +{
> + struct inject_priv_data *data = xmalloc(sizeof(struct inject_priv_data));
Overlength line.
> + data->inject_vec = xcalloc(SUPPORTED_PERSONALITIES,
> + sizeof(struct inject_opts *));
> + unsigned int p;
> + for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> + data->inject_vec[p] = xcalloc(nsyscall_vec[p],
> + sizeof(struct inject_opts));
> + }
> + data->opts.first = 1;
> + data->opts.step = 1;
> + data->opts.rval = INJECT_OPTS_RVAL_DEFAULT;
> + data->opts.signo = 0;
> + if (!str) {
> + if (fault_tokens_only) {
> + data->opts.rval = -ENOSYS;
> + return data;
> + } else {
> + error_msg_and_die("inject argument is required");
> + }
> + }
> + char *buf = xstrdup(str);
> + char *saveptr = NULL;
> + char *token;
> + for (token = strtok_r(buf, ";", &saveptr); token;
> + token = strtok_r(NULL, ";", &saveptr)) {
> + if (!parse_inject_token(token, &data->opts, fault_tokens_only))
> + error_msg_and_die("invalid %s argument '%s'",
> + description, token);
> + }
> + free(buf);
What bothers me is that this part is essentially the same as
parse_inject_expression (with few minor discrepancies). Is it possible
to refactor it to avoid code duplication?
> + if (data->opts.rval == INJECT_OPTS_RVAL_DEFAULT && !data->opts.signo) {
> + if (fault_tokens_only) {
> + data->opts.rval = -ENOSYS;
> + } else {
> + error_msg_and_die("invalid %s arguments '%s'",
> + description, str);
> + }
> + }
> + return data;
> +}
> +
> +void *
> parse_inject(const char *str)
> {
> - return NULL;
> + return parse_inject_common_args(str, false, "inject");
> }
> +
> void free_inject(void *_priv_data)
> {
> - struct inject_opts **vec = (struct inject_opts **)_priv_data;
> + struct inject_priv_data *data = (struct inject_priv_data *)_priv_data;
> unsigned int p;
> for (p = 0; p < SUPPORTED_PERSONALITIES; ++p)
> - if (vec[p])
> - free(vec[p]);
> - free(vec);
> + if (data->inject_vec[p])
> + free(data->inject_vec[p]);
> + free(data->inject_vec);
> + free(data);
> }
> +
> void
> apply_read(struct tcb *tcp, void *_priv_data)
> {
> tcp->qual_flg |= QUAL_READ;
> }
> +
> void
> apply_write(struct tcb *tcp, void *_priv_data)
> {
> tcp->qual_flg |= QUAL_WRITE;
> }
> +
> void
> apply_raw(struct tcb *tcp, void *_priv_data)
> {
> tcp->qual_flg |= QUAL_RAW;
> }
> +
> void
> apply_abbrev(struct tcb *tcp, void *_priv_data)
> {
> tcp->qual_flg |= QUAL_ABBREV;
> }
> +
> void
> apply_verbose(struct tcb *tcp, void *_priv_data)
> {
> tcp->qual_flg |= QUAL_VERBOSE;
> }
> +
> +void *
> +parse_fault(const char *str)
> +{
> + return parse_inject_common_args(str, true, "fault");
> +}
> +
> +void
> +apply_fault(struct tcb *tcp, void *_priv_data)
> +{
> + apply_inject(tcp, _priv_data);
> +}
> +
> +void free_fault(void *_priv_data) {
> + free_inject(_priv_data);
> +}
> diff --git a/defs.h b/defs.h
> index 9242b16..21dec65 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -203,6 +203,7 @@ struct inject_opts {
> uint16_t step;
> uint16_t signo;
> int rval;
> + bool init;
> };
>
> #define MAX_ERRNO_VALUE 4095
> @@ -970,7 +971,6 @@ extern unsigned nioctlents;
>
> extern const unsigned int nsyscall_vec[SUPPORTED_PERSONALITIES];
> extern const struct_sysent *const sysent_vec[SUPPORTED_PERSONALITIES];
> -extern struct inject_opts *inject_vec[SUPPORTED_PERSONALITIES];
>
> #ifdef IN_MPERS_BOOTSTRAP
> /* Transform multi-line MPERS_PRINTER_DECL statements to one-liners. */
> diff --git a/filter.h b/filter.h
> index ed435c8..7e03d75 100644
> --- a/filter.h
> +++ b/filter.h
> @@ -41,6 +41,8 @@ extern bool is_traced(struct tcb *);
> typedef int (*string_to_uint_func)(const char *);
> void parse_set(const char *const, struct number_set *const,
> string_to_uint_func, const char *const);
> +bool parse_inject_token(const char *const, struct inject_opts *const,
> + const bool);
>
> struct filter* add_filter_to_array(struct filter **, unsigned int *nfilters,
> const char *name);
> diff --git a/filter_action.c b/filter_action.c
> index f71f140..fd3fa92 100644
> --- a/filter_action.c
> +++ b/filter_action.c
> @@ -34,6 +34,7 @@ apply_ ## name(struct tcb *, void *)
>
> DECL_FILTER_ACTION(trace);
> DECL_FILTER_ACTION(inject);
> +DECL_FILTER_ACTION(fault);
> DECL_FILTER_ACTION(read);
> DECL_FILTER_ACTION(write);
> DECL_FILTER_ACTION(raw);
> @@ -50,6 +51,7 @@ free_ ## name(void *)
>
> DECL_FILTER_ACTION_PARSER(null);
> DECL_FILTER_ACTION_PARSER(inject);
> +DECL_FILTER_ACTION_PARSER(fault);
>
> #undef DECL_FILTER_ACTION_PARSER
>
> @@ -68,6 +70,7 @@ static const struct filter_action_type {
> } action_types[] = {
> FILTER_ACTION_TYPE(trace, 2, null, NULL),
> FILTER_ACTION_TYPE(inject, 2, inject, NULL),
> + FILTER_ACTION_TYPE(fault, 2, fault, NULL),
> FILTER_ACTION_TYPE(read, 1, null, is_traced),
> FILTER_ACTION_TYPE(write, 1, null, is_traced),
> FILTER_ACTION_TYPE(raw, 1, null, is_traced),
> diff --git a/filter_qualify.c b/filter_qualify.c
> index 6020ce8..79c1202 100644
> --- a/filter_qualify.c
> +++ b/filter_qualify.c
> @@ -80,7 +80,7 @@ sigstr_to_uint(const char *s)
> return -1;
> }
>
> -static bool
> +bool
> parse_inject_token(const char *const token, struct inject_opts *const fopts,
> const bool fault_tokens_only)
> {
> @@ -233,6 +233,11 @@ parse_raw(const char *const str)
> set_qualify_mode(action);
> }
>
> +struct inject_priv_data {
> + struct inject_opts opts;
> + struct inject_opts **inject_vec;
> +};
> +
> static void
> parse_inject_common(const char *const str,
> const bool fault_tokens_only,
> @@ -242,7 +247,8 @@ parse_inject_common(const char *const str,
> .first = 1,
> .step = 1,
> .rval = INJECT_OPTS_RVAL_DEFAULT,
> - .signo = 0
> + .signo = 0,
> + .init = true
> };
> char *buf = NULL;
> char *name = parse_inject_expression(str, &buf, &opts, fault_tokens_only);
> @@ -265,35 +271,19 @@ parse_inject_common(const char *const str,
> struct filter *filter = create_filter(action, "syscall");
> parse_filter(filter, name, description);
> set_qualify_mode(action);
> -
> free(buf);
> - const struct number_set *tmp_set = (const struct number_set *)
> - get_filter_priv_data(filter);
> - struct inject_opts **inject_vec = xcalloc(SUPPORTED_PERSONALITIES,
> - sizeof(struct inject_opts *));
> - /*
> - * Initialize inject_vec accourding to tmp_set.
> - */
> +
> + struct inject_priv_data *data = xmalloc(sizeof(struct inject_priv_data));
> + data->opts = opts;
> + data->inject_vec = xcalloc(SUPPORTED_PERSONALITIES,
> + sizeof(struct inject_opts *));
>
> unsigned int p;
> for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> - if (is_empty(&tmp_set[p])) {
> - continue;
> - }
> -
> - if (!inject_vec[p]) {
> - inject_vec[p] = xcalloc(nsyscall_vec[p],
> - sizeof(*inject_vec[p]));
> - }
> -
> - unsigned int i;
> - for (i = 0; i < nsyscall_vec[p]; ++i) {
> - if (is_number_in_set(i, &tmp_set[p])) {
> - inject_vec[p][i] = opts;
> - }
> - }
> + data->inject_vec[p] = xcalloc(nsyscall_vec[p],
> + sizeof(struct inject_opts));
> }
> - set_filter_action_priv_data(action, inject_vec);
> + set_filter_action_priv_data(action, data);
> }
>
> static void
Why did you decide to split patches this way, i.e., provide part of the
implementation in the previous patch (like "inject" action) and part
(like "fault" action) here?
More information about the Strace-devel
mailing list