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?
------------------------------------------------------------------------------
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
