Re: [PATCH v2 3/3] trace: Forbid dynamic field width in event format

2019-11-18 Thread Eric Blake

On 11/8/19 10:07 AM, Eric Blake wrote:

On 11/8/19 8:40 AM, Philippe Mathieu-Daudé wrote:

Since not all trace backends support dynamic field width in
format (dtrace via stap does not), forbid them.

Add a check to refuse field width in new formats:




+++ b/scripts/tracetool/__init__.py
@@ -206,6 +206,7 @@ class Event(object):
    "\s*"
    "(?:(?:(?P\".+),)?\s*(?P\".+))?"
    "\s*")
+    _DFWRE = re.compile(".*(%0?\*).*")


The use of leading and trailing .* is pointless if the RE itself is used 
unanchored and where you only care if the () matched.


Following up on this (based on an IRC conversation):

re.match _is_ anchored by default, while re.search is unanchored



This doesn't catch all valid uses of * in a printf format.  Better might 
be something like:


_DFWRE = re.compile("%[\d\.\- +#']*\*")

which matches only if there is a %, any number of characters that can 
form a printf flag, as well as a numeric field width but dynamic precision.




+    if Event._DFWRE.match(fmt):


Or put differently, if you want to drop the .* from the compiled regex, 
then you need to use .search(fmt) here rather than .match(fmt).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




Re: [PATCH v2 3/3] trace: Forbid dynamic field width in event format

2019-11-08 Thread Eric Blake

On 11/8/19 8:40 AM, Philippe Mathieu-Daudé wrote:

Since not all trace backends support dynamic field width in
format (dtrace via stap does not), forbid them.

Add a check to refuse field width in new formats:




+++ b/scripts/tracetool/__init__.py
@@ -206,6 +206,7 @@ class Event(object):
"\s*"
"(?:(?:(?P\".+),)?\s*(?P\".+))?"
"\s*")
+_DFWRE = re.compile(".*(%0?\*).*")


The use of leading and trailing .* is pointless if the RE itself is used 
unanchored and where you only care if the () matched.


This doesn't catch all valid uses of * in a printf format.  Better might 
be something like:


_DFWRE = re.compile("%[\d\.\- +#']*\*")

which matches only if there is a %, any number of characters that can 
form a printf flag, as well as a numeric field width but dynamic precision.




+if Event._DFWRE.match(fmt):
+raise ValueError("Event format must not contain field width '%*'")
  
  if len(fmt_trans) > 0:

  fmt = [fmt_trans, fmt]



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




[PATCH v2 3/3] trace: Forbid dynamic field width in event format

2019-11-08 Thread Philippe Mathieu-Daudé
Since not all trace backends support dynamic field width in
format (dtrace via stap does not), forbid them.

Add a check to refuse field width in new formats:

  $ make
  [...]
GEN hw/block/trace.h
  Traceback (most recent call last):
File "scripts/tracetool.py", line 152, in 
  main(sys.argv)
File "scripts/tracetool.py", line 143, in main
  events.extend(tracetool.read_events(fh, arg))
File "scripts/tracetool/__init__.py", line 371, in read_events
  event = Event.build(line)
File "scripts/tracetool/__init__.py", line 285, in build
  raise ValueError("Event format must not contain field width '%*'")
  ValueError: Error at hw/block/trace-events:11: Event format must not contain 
field width '%*'

Reported-by: Eric Blake 
Buglink: https://bugs.launchpad.net/qemu/+bug/1844817
Signed-off-by: Philippe Mathieu-Daudé 
---
 scripts/tracetool/__init__.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 44c118bc2a..e239be602b 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -206,6 +206,7 @@ class Event(object):
   "\s*"
   "(?:(?:(?P\".+),)?\s*(?P\".+))?"
   "\s*")
+_DFWRE = re.compile(".*(%0?\*).*")
 
 _VALID_PROPS = set(["disable", "tcg", "tcg-trans", "tcg-exec", "vcpu"])
 
@@ -280,6 +281,8 @@ class Event(object):
 if fmt.endswith(r'\n"'):
 raise ValueError("Event format must not end with a newline "
  "character")
+if Event._DFWRE.match(fmt):
+raise ValueError("Event format must not contain field width '%*'")
 
 if len(fmt_trans) > 0:
 fmt = [fmt_trans, fmt]
-- 
2.21.0