[PATCH v2] Implement -e trace=/REGEX/ option

[Dmitry V. Levin]

On Tue, Apr 11, 2017 at 08:27:12AM +0800, JingPiao Chen wrote:
> * qualify.c (qualify_syscall_regex): New function.
> (qualify_syscall_name): Use qualify_syscall_regex function.
> * tests/regex.test: Check this.
> * tests/Makefile.am (DECODER_TESTS): Add regex.test.

Great.  It definitely requires a piece of documentation in strace.1
and a NEWS entry.

> ---
>  qualify.c         | 36 +++++++++++++++++++++++++++
>  tests/Makefile.am |  1 +
>  tests/regex.test  | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 111 insertions(+)
>  create mode 100755 tests/regex.test
> 
> diff --git a/qualify.c b/qualify.c
> index 157d313..783bb4a 100644
> --- a/qualify.c
> +++ b/qualify.c
> @@ -27,6 +27,7 @@
>  
>  #include "defs.h"
>  #include "nsig.h"
> +#include <regex.h>
>  
>  typedef unsigned int number_slot_t;
>  #define BITS_PER_SLOT (sizeof(number_slot_t) * 8)
> @@ -258,6 +259,40 @@ qualify_syscall_class(const char *s, struct number_set *set)
>  }
>  
>  static bool
> +qualify_syscall_regex(const char *s, struct number_set *set)
> +{
> +	int len = strlen(s);
> +
> +	if(!s || s[0] != '/' || s[len - 1] != '/')
> +		return false;
> +
> +	char *copy = xstrdup(s);
> +	copy[len - 1] = '\0';

I do think that trace=/REGEX/ has some unhealthy redundancy.
The leading slash is enough.  Let it be trace=/REGEX instead.

It would simplify the code, too.

> +	regex_t preg;
> +	if (regcomp(&preg, copy + 1, REG_EXTENDED | REG_NOSUB))
> +		error_msg_and_die("invalid regular expression: '%s'", copy + 1);

Please use regerror() to make error diagnostics more descriptive.

> +	free(copy);
> +
> +	unsigned int p;
> +	for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> +		unsigned int i;
> +
> +		for (i = 0; i < nsyscall_vec[p]; ++i) {
> +			if (!sysent_vec[p][i].sys_name
> +			    || regexec(&preg, sysent_vec[p][i].sys_name,
> +				       0, NULL, 0)) {
> +				continue;

regexec can return values other than zero or REG_NOMATCH, they are errors
that have to be handled.

> +			}
> +			add_number_to_set(i, &set[p]);
> +		}
> +	}
> +
> +	regfree(&preg);
> +	return true;
> +}

qualify_syscall_regex should have the same "return found" semantics as
qualify_syscall_name, so a regular expression that matches no syscalls
would be a syntax error.

> +static bool
>  qualify_syscall_name(const char *s, struct number_set *set)
>  {
>  	unsigned int p;
> @@ -285,6 +320,7 @@ qualify_syscall(const char *token, struct number_set *set)
>  	if (*token >= '0' && *token <= '9')
>  		return qualify_syscall_number(token, set);
>  	return qualify_syscall_class(token, set)
> +	       || qualify_syscall_regex(token, set)
>  	       || qualify_syscall_name(token, set);

As there are no class or syscall name that start with a slash,
I think it should be

	if (*token == '/')
		return qualify_syscall_regex(token, set);

[...]
> --- /dev/null
> +++ b/tests/regex.test
> @@ -0,0 +1,74 @@
> +#!/bin/sh
> +#
> +# Check -e trace=/REGEX/ option.

Please also add a check for an invalid regexp and a regexp that would
never match a syscall.


-- 
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/20170412/ed83b68b/attachment.bin>