QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Qemu-devel] [PATCH 0/2] trace: Forbid trailing newline in event format
@ 2019-09-13 10:52 Philippe Mathieu-Daudé
  2019-09-13 10:52 ` [Qemu-devel] [PATCH 1/2] trace: Remove trailing newline in events Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-13 10:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, qemu-block, Philippe Mathieu-Daudé,
	Jason Wang, Stefan Hajnoczi, Paolo Bonzini

Hi Stefan,

I'v been confused by trailing newline in trace reports,
so this series aims to fix this, by cleaning current
formats and add a check to catch new one introduced.

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  trace: Remove trailing newline in events
  trace: Forbid event format ending with newline character

 docs/devel/tracing.txt        |  2 ++
 hw/misc/trace-events          | 10 +++++-----
 hw/scsi/trace-events          |  2 +-
 hw/sd/trace-events            |  2 +-
 nbd/trace-events              |  4 ++--
 net/trace-events              |  6 +++---
 scripts/tracetool/__init__.py |  3 +++
 7 files changed, 17 insertions(+), 12 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/2] trace: Remove trailing newline in events
  2019-09-13 10:52 [Qemu-devel] [PATCH 0/2] trace: Forbid trailing newline in event format Philippe Mathieu-Daudé
@ 2019-09-13 10:52 ` Philippe Mathieu-Daudé
  2019-09-13 10:52 ` [Qemu-devel] [PATCH 2/2] trace: Forbid event format ending with newline character Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-13 10:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, qemu-block, Philippe Mathieu-Daudé,
	Jason Wang, Stefan Hajnoczi, Paolo Bonzini

While the tracing frawework does not forbid trailing newline in
events format string, using them lead to confuse output.
It is the responsibility of the backend to properly end an event
line.

Some of our formats have trailing newlines, remove them.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/misc/trace-events | 10 +++++-----
 hw/scsi/trace-events |  2 +-
 hw/sd/trace-events   |  2 +-
 nbd/trace-events     |  4 ++--
 net/trace-events     |  6 +++---
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index c1ea1aa437..74276225f8 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -118,11 +118,11 @@ iotkit_secctl_ns_read(uint32_t offset, uint64_t data, unsigned size) "IoTKit Sec
 iotkit_secctl_ns_write(uint32_t offset, uint64_t data, unsigned size) "IoTKit SecCtl NS regs write: offset 0x%x data 0x%" PRIx64 " size %u"
 
 # imx6ul_ccm.c
-ccm_entry(void) "\n"
-ccm_freq(uint32_t freq) "freq = %d\n"
-ccm_clock_freq(uint32_t clock, uint32_t freq) "(Clock = %d) = %d\n"
-ccm_read_reg(const char *reg_name, uint32_t value) "reg[%s] <= 0x%" PRIx32 "\n"
-ccm_write_reg(const char *reg_name, uint32_t value) "reg[%s] => 0x%" PRIx32 "\n"
+ccm_entry(void) ""
+ccm_freq(uint32_t freq) "freq = %d"
+ccm_clock_freq(uint32_t clock, uint32_t freq) "(Clock = %d) = %d"
+ccm_read_reg(const char *reg_name, uint32_t value) "reg[%s] <= 0x%" PRIx32
+ccm_write_reg(const char *reg_name, uint32_t value) "reg[%s] => 0x%" PRIx32
 
 # iotkit-sysinfo.c
 iotkit_sysinfo_read(uint64_t offset, uint64_t data, unsigned size) "IoTKit SysInfo read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index 452b5994e6..b0820052f8 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -28,7 +28,7 @@ mptsas_mmio_read(void *dev, uint32_t addr, uint32_t val) "dev %p addr 0x%08x val
 mptsas_mmio_unhandled_read(void *dev, uint32_t addr) "dev %p addr 0x%08x"
 mptsas_mmio_unhandled_write(void *dev, uint32_t addr, uint32_t val) "dev %p addr 0x%08x value 0x%x"
 mptsas_mmio_write(void *dev, uint32_t addr, uint32_t val) "dev %p addr 0x%08x value 0x%x"
-mptsas_process_message(void *dev, int msg, uint32_t ctx) "dev %p cmd %d context 0x%08x\n"
+mptsas_process_message(void *dev, int msg, uint32_t ctx) "dev %p cmd %d context 0x%08x"
 mptsas_process_scsi_io_request(void *dev, int bus, int target, int lun, uint64_t len) "dev %p dev %d:%d:%d length %"PRIu64""
 mptsas_reset(void *dev) "dev %p "
 mptsas_scsi_overflow(void *dev, uint32_t ctx, uint64_t req, uint64_t found) "dev %p context 0x%08x: %"PRIu64"/%"PRIu64""
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 52971dc033..efcff666a2 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -4,7 +4,7 @@
 bcm2835_sdhost_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 bcm2835_sdhost_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u"
 bcm2835_sdhost_edm_change(const char *why, uint32_t edm) "(%s) EDM now 0x%x"
-bcm2835_sdhost_update_irq(uint32_t irq) "IRQ bits 0x%x\n"
+bcm2835_sdhost_update_irq(uint32_t irq) "IRQ bits 0x%x"
 
 # core.c
 sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg) "@%s CMD%02d arg 0x%08x"
diff --git a/nbd/trace-events b/nbd/trace-events
index f6cde96790..a955918e97 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -61,8 +61,8 @@ nbd_negotiate_begin(void) "Beginning negotiation"
 nbd_negotiate_new_style_size_flags(uint64_t size, unsigned flags) "advertising size %" PRIu64 " and flags 0x%x"
 nbd_negotiate_success(void) "Negotiation succeeded"
 nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from, uint32_t len) "Got request: { magic = 0x%" PRIx32 ", .flags = 0x%" PRIx16 ", .type = 0x%" PRIx16 ", from = %" PRIu64 ", len = %" PRIu32 " }"
-nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p\n"
-nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p\n"
+nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p"
+nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p"
 nbd_co_send_simple_reply(uint64_t handle, uint32_t error, const char *errname, int len) "Send simple reply: handle = %" PRIu64 ", error = %" PRIu32 " (%s), len = %d"
 nbd_co_send_structured_done(uint64_t handle) "Send structured reply done: handle = %" PRIu64
 nbd_co_send_structured_read(uint64_t handle, uint64_t offset, void *data, size_t size) "Send structured read data reply: handle = %" PRIu64 ", offset = %" PRIu64 ", data = %p, len = %zu"
diff --git a/net/trace-events b/net/trace-events
index ac57056497..02c13fd0ba 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -17,9 +17,9 @@ colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
 colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
 colo_old_packet_check_found(int64_t old_time) "%" PRId64
 colo_compare_miscompare(void) ""
-colo_compare_tcp_info(const char *pkt, uint32_t seq, uint32_t ack, int hdlen, int pdlen, int offset, int flags) "%s: seq/ack= %u/%u hdlen= %d pdlen= %d offset= %d flags=%d\n"
+colo_compare_tcp_info(const char *pkt, uint32_t seq, uint32_t ack, int hdlen, int pdlen, int offset, int flags) "%s: seq/ack= %u/%u hdlen= %d pdlen= %d offset= %d flags=%d"
 
 # filter-rewriter.c
 colo_filter_rewriter_debug(void) ""
-colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=0x%x\n"
-colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u\n"
+colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=0x%x"
+colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u"
-- 
2.20.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/2] trace: Forbid event format ending with newline character
  2019-09-13 10:52 [Qemu-devel] [PATCH 0/2] trace: Forbid trailing newline in event format Philippe Mathieu-Daudé
  2019-09-13 10:52 ` [Qemu-devel] [PATCH 1/2] trace: Remove trailing newline in events Philippe Mathieu-Daudé
@ 2019-09-13 10:52 ` Philippe Mathieu-Daudé
  2019-09-13 20:01   ` [Qemu-devel] [Qemu-block] " John Snow
  2019-09-13 20:10 ` [Qemu-devel] [Qemu-block] [PATCH 0/2] trace: Forbid trailing newline in event format John Snow
  2019-09-16  8:07 ` Kevin Wolf
  3 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-13 10:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, qemu-block, Philippe Mathieu-Daudé,
	Jason Wang, Stefan Hajnoczi, Paolo Bonzini

Event format ending with newlines confuse the trace reports.
Forbid them.

Add a check to refuse new format added with trailing newline:

  $ make
  [...]
    GEN     hw/misc/trace.h
  Traceback (most recent call last):
    File "scripts/tracetool.py", line 152, in <module>
      main(sys.argv)
    File "scripts/tracetool.py", line 143, in main
      events.extend(tracetool.read_events(fh, arg))
    File "scripts/tracetool/__init__.py", line 367, in read_events
      event = Event.build(line)
    File "scripts/tracetool/__init__.py", line 281, in build
      raise ValueError("Event format can not end with a newline character")
  ValueError: Error at hw/misc/trace-events:121: Event format can not end with a newline character

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 docs/devel/tracing.txt        | 2 ++
 scripts/tracetool/__init__.py | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
index 76e492a489..8231bbf5d1 100644
--- a/docs/devel/tracing.txt
+++ b/docs/devel/tracing.txt
@@ -112,6 +112,8 @@ Trace events should use types as follows:
 Format strings should reflect the types defined in the trace event.  Take
 special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
 respectively.  This ensures portability between 32- and 64-bit platforms.
+Format strings must not end with a newline character.  It is the responsibility
+of backends to adapt line ending for proper logging.
 
 Each event declaration will start with the event name, then its arguments,
 finally a format string for pretty-printing. For example:
diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 6fca674936..57df74e67c 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -277,6 +277,9 @@ class Event(object):
         if fmt.find("%m") != -1 or fmt_trans.find("%m") != -1:
             raise ValueError("Event format '%m' is forbidden, pass the error "
                              "as an explicit trace argument")
+        if fmt.endswith("\\n\""):
+            raise ValueError("Event format must not end with a newline "
+                             "character")
 
         if len(fmt_trans) > 0:
             fmt = [fmt_trans, fmt]
-- 
2.20.1



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] trace: Forbid event format ending with newline character
  2019-09-13 10:52 ` [Qemu-devel] [PATCH 2/2] trace: Forbid event format ending with newline character Philippe Mathieu-Daudé
@ 2019-09-13 20:01   ` " John Snow
  2019-09-13 21:00     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 8+ messages in thread
From: John Snow @ 2019-09-13 20:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, Jason Wang, Stefan Hajnoczi, qemu-block



On 9/13/19 6:52 AM, Philippe Mathieu-Daudé wrote:
> Event format ending with newlines confuse the trace reports.
> Forbid them.
> 
> Add a check to refuse new format added with trailing newline:
> 
>   $ make
>   [...]
>     GEN     hw/misc/trace.h
>   Traceback (most recent call last):
>     File "scripts/tracetool.py", line 152, in <module>
>       main(sys.argv)
>     File "scripts/tracetool.py", line 143, in main
>       events.extend(tracetool.read_events(fh, arg))
>     File "scripts/tracetool/__init__.py", line 367, in read_events
>       event = Event.build(line)
>     File "scripts/tracetool/__init__.py", line 281, in build
>       raise ValueError("Event format can not end with a newline character")
>   ValueError: Error at hw/misc/trace-events:121: Event format can not end with a newline character
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  docs/devel/tracing.txt        | 2 ++
>  scripts/tracetool/__init__.py | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
> index 76e492a489..8231bbf5d1 100644
> --- a/docs/devel/tracing.txt
> +++ b/docs/devel/tracing.txt
> @@ -112,6 +112,8 @@ Trace events should use types as follows:
>  Format strings should reflect the types defined in the trace event.  Take
>  special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
>  respectively.  This ensures portability between 32- and 64-bit platforms.
> +Format strings must not end with a newline character.  It is the responsibility
> +of backends to adapt line ending for proper logging.
>  
>  Each event declaration will start with the event name, then its arguments,
>  finally a format string for pretty-printing. For example:
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 6fca674936..57df74e67c 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -277,6 +277,9 @@ class Event(object):
>          if fmt.find("%m") != -1 or fmt_trans.find("%m") != -1:
>              raise ValueError("Event format '%m' is forbidden, pass the error "
>                               "as an explicit trace argument")
> +        if fmt.endswith("\\n\""):
> +            raise ValueError("Event format must not end with a newline "

It's barely worth mentioning, but you can use r"\n" for cases like this,
if it makes it easier to read.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH 0/2] trace: Forbid trailing newline in event format
  2019-09-13 10:52 [Qemu-devel] [PATCH 0/2] trace: Forbid trailing newline in event format Philippe Mathieu-Daudé
  2019-09-13 10:52 ` [Qemu-devel] [PATCH 1/2] trace: Remove trailing newline in events Philippe Mathieu-Daudé
  2019-09-13 10:52 ` [Qemu-devel] [PATCH 2/2] trace: Forbid event format ending with newline character Philippe Mathieu-Daudé
@ 2019-09-13 20:10 ` John Snow
  2019-09-16  8:07 ` Kevin Wolf
  3 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2019-09-13 20:10 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, Jason Wang, Stefan Hajnoczi, qemu-block



On 9/13/19 6:52 AM, Philippe Mathieu-Daudé wrote:
> Hi Stefan,
> 
> I'v been confused by trailing newline in trace reports,
> so this series aims to fix this, by cleaning current
> formats and add a check to catch new one introduced.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (2):
>   trace: Remove trailing newline in events
>   trace: Forbid event format ending with newline character
> 
>  docs/devel/tracing.txt        |  2 ++
>  hw/misc/trace-events          | 10 +++++-----
>  hw/scsi/trace-events          |  2 +-
>  hw/sd/trace-events            |  2 +-
>  nbd/trace-events              |  4 ++--
>  net/trace-events              |  6 +++---
>  scripts/tracetool/__init__.py |  3 +++
>  7 files changed, 17 insertions(+), 12 deletions(-)
> 

Never mind my bikeshedding.

Reviewed-by: John Snow <jsnow@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] trace: Forbid event format ending with newline character
  2019-09-13 20:01   ` [Qemu-devel] [Qemu-block] " John Snow
@ 2019-09-13 21:00     ` Philippe Mathieu-Daudé
  2019-09-13 21:08       ` John Snow
  0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-09-13 21:00 UTC (permalink / raw)
  To: John Snow, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, Jason Wang, Stefan Hajnoczi, qemu-block

On 9/13/19 10:01 PM, John Snow wrote:
> On 9/13/19 6:52 AM, Philippe Mathieu-Daudé wrote:
>> Event format ending with newlines confuse the trace reports.
>> Forbid them.
>>
>> Add a check to refuse new format added with trailing newline:
>>
>>   $ make
>>   [...]
>>     GEN     hw/misc/trace.h
>>   Traceback (most recent call last):
>>     File "scripts/tracetool.py", line 152, in <module>
>>       main(sys.argv)
>>     File "scripts/tracetool.py", line 143, in main
>>       events.extend(tracetool.read_events(fh, arg))
>>     File "scripts/tracetool/__init__.py", line 367, in read_events
>>       event = Event.build(line)
>>     File "scripts/tracetool/__init__.py", line 281, in build
>>       raise ValueError("Event format can not end with a newline character")
>>   ValueError: Error at hw/misc/trace-events:121: Event format can not end with a newline character
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  docs/devel/tracing.txt        | 2 ++
>>  scripts/tracetool/__init__.py | 3 +++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
>> index 76e492a489..8231bbf5d1 100644
>> --- a/docs/devel/tracing.txt
>> +++ b/docs/devel/tracing.txt
>> @@ -112,6 +112,8 @@ Trace events should use types as follows:
>>  Format strings should reflect the types defined in the trace event.  Take
>>  special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
>>  respectively.  This ensures portability between 32- and 64-bit platforms.
>> +Format strings must not end with a newline character.  It is the responsibility
>> +of backends to adapt line ending for proper logging.
>>  
>>  Each event declaration will start with the event name, then its arguments,
>>  finally a format string for pretty-printing. For example:
>> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
>> index 6fca674936..57df74e67c 100644
>> --- a/scripts/tracetool/__init__.py
>> +++ b/scripts/tracetool/__init__.py
>> @@ -277,6 +277,9 @@ class Event(object):
>>          if fmt.find("%m") != -1 or fmt_trans.find("%m") != -1:
>>              raise ValueError("Event format '%m' is forbidden, pass the error "
>>                               "as an explicit trace argument")
>> +        if fmt.endswith("\\n\""):
>> +            raise ValueError("Event format must not end with a newline "
> 
> It's barely worth mentioning, but you can use r"\n" for cases like this,
> if it makes it easier to read.

TIL Python r"" :)

This would be r"\n\"", right? We need to match the trailing '"'.
Same length, not sure which string is easier to review =)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] trace: Forbid event format ending with newline character
  2019-09-13 21:00     ` Philippe Mathieu-Daudé
@ 2019-09-13 21:08       ` John Snow
  0 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2019-09-13 21:08 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Fam Zheng, Paolo Bonzini, Jason Wang, Stefan Hajnoczi, qemu-block



On 9/13/19 5:00 PM, Philippe Mathieu-Daudé wrote:
> On 9/13/19 10:01 PM, John Snow wrote:
>> On 9/13/19 6:52 AM, Philippe Mathieu-Daudé wrote:
>>> Event format ending with newlines confuse the trace reports.
>>> Forbid them.
>>>
>>> Add a check to refuse new format added with trailing newline:
>>>
>>>   $ make
>>>   [...]
>>>     GEN     hw/misc/trace.h
>>>   Traceback (most recent call last):
>>>     File "scripts/tracetool.py", line 152, in <module>
>>>       main(sys.argv)
>>>     File "scripts/tracetool.py", line 143, in main
>>>       events.extend(tracetool.read_events(fh, arg))
>>>     File "scripts/tracetool/__init__.py", line 367, in read_events
>>>       event = Event.build(line)
>>>     File "scripts/tracetool/__init__.py", line 281, in build
>>>       raise ValueError("Event format can not end with a newline character")
>>>   ValueError: Error at hw/misc/trace-events:121: Event format can not end with a newline character
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  docs/devel/tracing.txt        | 2 ++
>>>  scripts/tracetool/__init__.py | 3 +++
>>>  2 files changed, 5 insertions(+)
>>>
>>> diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
>>> index 76e492a489..8231bbf5d1 100644
>>> --- a/docs/devel/tracing.txt
>>> +++ b/docs/devel/tracing.txt
>>> @@ -112,6 +112,8 @@ Trace events should use types as follows:
>>>  Format strings should reflect the types defined in the trace event.  Take
>>>  special care to use PRId64 and PRIu64 for int64_t and uint64_t types,
>>>  respectively.  This ensures portability between 32- and 64-bit platforms.
>>> +Format strings must not end with a newline character.  It is the responsibility
>>> +of backends to adapt line ending for proper logging.
>>>  
>>>  Each event declaration will start with the event name, then its arguments,
>>>  finally a format string for pretty-printing. For example:
>>> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
>>> index 6fca674936..57df74e67c 100644
>>> --- a/scripts/tracetool/__init__.py
>>> +++ b/scripts/tracetool/__init__.py
>>> @@ -277,6 +277,9 @@ class Event(object):
>>>          if fmt.find("%m") != -1 or fmt_trans.find("%m") != -1:
>>>              raise ValueError("Event format '%m' is forbidden, pass the error "
>>>                               "as an explicit trace argument")
>>> +        if fmt.endswith("\\n\""):
>>> +            raise ValueError("Event format must not end with a newline "
>>
>> It's barely worth mentioning, but you can use r"\n" for cases like this,
>> if it makes it easier to read.
> 
> TIL Python r"" :)
> 
> This would be r"\n\"", right? We need to match the trailing '"'.
> Same length, not sure which string is easier to review =)
> 

Ah, wow, I did misread this. It's confusing!

In this case, you can use r'\n"' to get the job done.

(My r-b stands, anyway.)

--js


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH 0/2] trace: Forbid trailing newline in event format
  2019-09-13 10:52 [Qemu-devel] [PATCH 0/2] trace: Forbid trailing newline in event format Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2019-09-13 20:10 ` [Qemu-devel] [Qemu-block] [PATCH 0/2] trace: Forbid trailing newline in event format John Snow
@ 2019-09-16  8:07 ` Kevin Wolf
  3 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2019-09-16  8:07 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Fam Zheng, qemu-block, Jason Wang, qemu-devel, Stefan Hajnoczi,
	Paolo Bonzini

Am 13.09.2019 um 12:52 hat Philippe Mathieu-Daudé geschrieben:
> Hi Stefan,
> 
> I'v been confused by trailing newline in trace reports,
> so this series aims to fix this, by cleaning current
> formats and add a check to catch new one introduced.

Good idea.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-13 10:52 [Qemu-devel] [PATCH 0/2] trace: Forbid trailing newline in event format Philippe Mathieu-Daudé
2019-09-13 10:52 ` [Qemu-devel] [PATCH 1/2] trace: Remove trailing newline in events Philippe Mathieu-Daudé
2019-09-13 10:52 ` [Qemu-devel] [PATCH 2/2] trace: Forbid event format ending with newline character Philippe Mathieu-Daudé
2019-09-13 20:01   ` [Qemu-devel] [Qemu-block] " John Snow
2019-09-13 21:00     ` Philippe Mathieu-Daudé
2019-09-13 21:08       ` John Snow
2019-09-13 20:10 ` [Qemu-devel] [Qemu-block] [PATCH 0/2] trace: Forbid trailing newline in event format John Snow
2019-09-16  8:07 ` Kevin Wolf

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org qemu-devel@archiver.kernel.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox