| Message ID | 20210105191721.120463-3-lvivier@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tracetool: fix log-stap format | expand |
On 1/5/21 8:17 PM, Laurent Vivier wrote: > macro is not reset after use, so the format decoded is always the > one of the first "PRI" in the format string. > > For instance: > > vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size, \ > uint32_t flags) "dev: %p offset: %"PRIu32" \ > size: %"PRIu32" flags: 0x%"PRIx32 > > generates: > > printf("%d@%d vhost_vdpa_set_config dev: %p offset: %u size: %u \ > flags: 0x%u\n", pid(), gettimeofday_ns(), dev, offset, \ > size, flags) > > for the "flags" parameter, we can see a "0x%u" rather than a "0x%x" > because the first macro was "PRIu32" (for offset). > > In the loop, macro becomes "PRIu32PRIu32PRIx32", and c_macro_to_format() > returns always macro[3] ('u' in this case). This patch resets macro after > the format has been decoded. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > scripts/tracetool/format/log_stap.py | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/scripts/tracetool/format/log_stap.py b/scripts/tracetool/format/log_stap.py > index b486beb67239..3e1186ae9cc2 100644 > --- a/scripts/tracetool/format/log_stap.py > +++ b/scripts/tracetool/format/log_stap.py > @@ -54,6 +54,7 @@ def c_fmt_to_stap(fmt): > else: > if state == STATE_MACRO: > bits.append(c_macro_to_format(macro)) > + macro = "" > state = STATE_LITERAL > elif fmt[i] == ' ' or fmt[i] == '\t': > if state == STATE_MACRO: What about the 'else' case?
On 05/01/2021 20:44, Philippe Mathieu-Daudé wrote: > On 1/5/21 8:17 PM, Laurent Vivier wrote: >> macro is not reset after use, so the format decoded is always the >> one of the first "PRI" in the format string. >> >> For instance: >> >> vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size, \ >> uint32_t flags) "dev: %p offset: %"PRIu32" \ >> size: %"PRIu32" flags: 0x%"PRIx32 >> >> generates: >> >> printf("%d@%d vhost_vdpa_set_config dev: %p offset: %u size: %u \ >> flags: 0x%u\n", pid(), gettimeofday_ns(), dev, offset, \ >> size, flags) >> >> for the "flags" parameter, we can see a "0x%u" rather than a "0x%x" >> because the first macro was "PRIu32" (for offset). >> >> In the loop, macro becomes "PRIu32PRIu32PRIx32", and c_macro_to_format() >> returns always macro[3] ('u' in this case). This patch resets macro after >> the format has been decoded. >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> scripts/tracetool/format/log_stap.py | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/scripts/tracetool/format/log_stap.py b/scripts/tracetool/format/log_stap.py >> index b486beb67239..3e1186ae9cc2 100644 >> --- a/scripts/tracetool/format/log_stap.py >> +++ b/scripts/tracetool/format/log_stap.py >> @@ -54,6 +54,7 @@ def c_fmt_to_stap(fmt): >> else: >> if state == STATE_MACRO: >> bits.append(c_macro_to_format(macro)) >> + macro = "" >> state = STATE_LITERAL >> elif fmt[i] == ' ' or fmt[i] == '\t': >> if state == STATE_MACRO: > > What about the 'else' case? > It already has the 'macro = ""', so no need to fix it. Thanks, Laurent
On Tue, Jan 05, 2021 at 08:17:21PM +0100, Laurent Vivier wrote: > macro is not reset after use, so the format decoded is always the > one of the first "PRI" in the format string. > > For instance: > > vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size, \ > uint32_t flags) "dev: %p offset: %"PRIu32" \ > size: %"PRIu32" flags: 0x%"PRIx32 > > generates: > > printf("%d@%d vhost_vdpa_set_config dev: %p offset: %u size: %u \ > flags: 0x%u\n", pid(), gettimeofday_ns(), dev, offset, \ > size, flags) > > for the "flags" parameter, we can see a "0x%u" rather than a "0x%x" > because the first macro was "PRIu32" (for offset). > > In the loop, macro becomes "PRIu32PRIu32PRIx32", and c_macro_to_format() > returns always macro[3] ('u' in this case). This patch resets macro after > the format has been decoded. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > scripts/tracetool/format/log_stap.py | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/scripts/tracetool/format/log_stap.py b/scripts/tracetool/format/log_stap.py > index b486beb67239..3e1186ae9cc2 100644 > --- a/scripts/tracetool/format/log_stap.py > +++ b/scripts/tracetool/format/log_stap.py > @@ -54,6 +54,7 @@ def c_fmt_to_stap(fmt): > else: > if state == STATE_MACRO: > bits.append(c_macro_to_format(macro)) > + macro = "" > state = STATE_LITERAL > elif fmt[i] == ' ' or fmt[i] == '\t': > if state == STATE_MACRO: Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
On Tue, Jan 05, 2021 at 08:17:21PM +0100, Laurent Vivier wrote: > macro is not reset after use, so the format decoded is always the > one of the first "PRI" in the format string. > > For instance: > > vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size, \ > uint32_t flags) "dev: %p offset: %"PRIu32" \ > size: %"PRIu32" flags: 0x%"PRIx32 > > generates: > > printf("%d@%d vhost_vdpa_set_config dev: %p offset: %u size: %u \ > flags: 0x%u\n", pid(), gettimeofday_ns(), dev, offset, \ > size, flags) > > for the "flags" parameter, we can see a "0x%u" rather than a "0x%x" > because the first macro was "PRIu32" (for offset). > > In the loop, macro becomes "PRIu32PRIu32PRIx32", and c_macro_to_format() > returns always macro[3] ('u' in this case). This patch resets macro after > the format has been decoded. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > scripts/tracetool/format/log_stap.py | 1 + > 1 file changed, 1 insertion(+) Thanks, applied to my tracing tree: https://gitlab.com/stefanha/qemu/commits/tracing Stefan
diff --git a/scripts/tracetool/format/log_stap.py b/scripts/tracetool/format/log_stap.py index b486beb67239..3e1186ae9cc2 100644 --- a/scripts/tracetool/format/log_stap.py +++ b/scripts/tracetool/format/log_stap.py @@ -54,6 +54,7 @@ def c_fmt_to_stap(fmt): else: if state == STATE_MACRO: bits.append(c_macro_to_format(macro)) + macro = "" state = STATE_LITERAL elif fmt[i] == ' ' or fmt[i] == '\t': if state == STATE_MACRO:
macro is not reset after use, so the format decoded is always the one of the first "PRI" in the format string. For instance: vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size, \ uint32_t flags) "dev: %p offset: %"PRIu32" \ size: %"PRIu32" flags: 0x%"PRIx32 generates: printf("%d@%d vhost_vdpa_set_config dev: %p offset: %u size: %u \ flags: 0x%u\n", pid(), gettimeofday_ns(), dev, offset, \ size, flags) for the "flags" parameter, we can see a "0x%u" rather than a "0x%x" because the first macro was "PRIu32" (for offset). In the loop, macro becomes "PRIu32PRIu32PRIx32", and c_macro_to_format() returns always macro[3] ('u' in this case). This patch resets macro after the format has been decoded. Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- scripts/tracetool/format/log_stap.py | 1 + 1 file changed, 1 insertion(+)