[v2,6/6] trace: replace hand-crafted pattern_glob with g_pattern_match_simple

Message ID 20191205122518.10010-7-alex.bennee@linaro.org
State Superseded
Headers show
Series
  • linux-user mmap debug cleanup
Related show

Commit Message

Alex Bennée Dec. 5, 2019, 12:25 p.m.
We already use g_pattern_match elsewhere so remove the duplication.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 trace/control.c | 35 +----------------------------------
 1 file changed, 1 insertion(+), 34 deletions(-)

-- 
2.20.1

Comments

Richard Henderson Dec. 5, 2019, 3:51 p.m. | #1
On 12/5/19 4:25 AM, Alex Bennée wrote:
> We already use g_pattern_match elsewhere so remove the duplication.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> ---

>  trace/control.c | 35 +----------------------------------

>  1 file changed, 1 insertion(+), 34 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



r~
Stefan Hajnoczi Dec. 6, 2019, 11:03 a.m. | #2
On Thu, Dec 05, 2019 at 12:25:17PM +0000, Alex Bennée wrote:
> We already use g_pattern_match elsewhere so remove the duplication.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> ---

>  trace/control.c | 35 +----------------------------------

>  1 file changed, 1 insertion(+), 34 deletions(-)


Is g_pattern_match() a superset of pattern_glob()?  Existing patterns
should continue to work.

Stefan
Alex Bennée Dec. 6, 2019, 11:59 a.m. | #3
Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Thu, Dec 05, 2019 at 12:25:17PM +0000, Alex Bennée wrote:

>> We already use g_pattern_match elsewhere so remove the duplication.

>>

>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

>> ---

>>  trace/control.c | 35 +----------------------------------

>>  1 file changed, 1 insertion(+), 34 deletions(-)

>

> Is g_pattern_match() a superset of pattern_glob()?  Existing patterns

> should continue to work.


Yes - it supports more than pattern_glob and a bit less than the system
glob():

  The g_pattern_match* functions match a string against a pattern
  containing '*' and '?' wildcards with similar semantics as the standard
  glob() function: '*' matches an arbitrary, possibly empty, string, '?'
  matches an arbitrary character.

  Note that in contrast to glob(), the '/' character can be matched by the
  wildcards, there are no '[...]' character ranges and '*' and '?' can not
  be escaped to include them literally in a pattern.

If you give me some example existing pattern forms we can add them to
test-logging. I manually tested both single and double * patterns while
working on the rest of the series.

>

> Stefan



--
Alex Bennée
Stefan Hajnoczi Dec. 9, 2019, 3:58 p.m. | #4
On Thu, Dec 05, 2019 at 12:25:17PM +0000, Alex Bennée wrote:
> We already use g_pattern_match elsewhere so remove the duplication.

> 

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> ---

>  trace/control.c | 35 +----------------------------------

>  1 file changed, 1 insertion(+), 34 deletions(-)


Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Patch

diff --git a/trace/control.c b/trace/control.c
index d9cafc161bb..0fb81241607 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -98,38 +98,6 @@  TraceEvent *trace_event_name(const char *name)
     return NULL;
 }
 
-static bool pattern_glob(const char *pat, const char *ev)
-{
-    while (*pat != '\0' && *ev != '\0') {
-        if (*pat == *ev) {
-            pat++;
-            ev++;
-        }
-        else if (*pat == '*') {
-            if (pattern_glob(pat, ev+1)) {
-                return true;
-            } else if (pattern_glob(pat+1, ev)) {
-                return true;
-            } else {
-                return false;
-            }
-        } else {
-            return false;
-        }
-    }
-
-    while (*pat == '*') {
-        pat++;
-    }
-
-    if (*pat == '\0' && *ev == '\0') {
-        return true;
-    } else {
-        return false;
-    }
-}
-
-
 void trace_event_iter_init(TraceEventIter *iter, const char *pattern)
 {
     iter->event = 0;
@@ -148,8 +116,7 @@  TraceEvent *trace_event_iter_next(TraceEventIter *iter)
             iter->group++;
         }
         if (!iter->pattern ||
-            pattern_glob(iter->pattern,
-                         trace_event_get_name(ev))) {
+            g_pattern_match_simple(iter->pattern, trace_event_get_name(ev))) {
             return ev;
         }
     }