Re: [PATCH v3 5/8] tracetool: support "-" as a shorthand for stdout

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Aug 07, 2025 at 03:46:08PM -0400, Stefan Hajnoczi wrote:
> On Wed, Aug 06, 2025 at 05:48:29PM +0100, Daniel P. Berrangé wrote:
> > This avoids callers needing to use the UNIX-only /dev/stdout
> > workaround.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  scripts/tracetool/__init__.py | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> > index 0f33758870..c8fd3a7ddc 100644
> > --- a/scripts/tracetool/__init__.py
> > +++ b/scripts/tracetool/__init__.py
> > @@ -38,8 +38,12 @@ def error(*lines):
> >  
> >  def out_open(filename):
> >      global out_filename, out_fobj
> > -    out_filename = posix_relpath(filename)
> > -    out_fobj = open(filename, 'wt')
> > +    if filename == "-":
> > +        out_filename = "[stdout]"
> 
> A few lines above:
> 
>   out_filename = '<none>'
>   out_fobj = sys.stdout
> 
> Stick to '<none>' here for consistency?

Curious - that suggests that it was intended to be able to write to
stdout by default, but tracetool.py unconditionally calls out_open()
so those default assignments are effectively dead code, unless this
internal code is called by something other than the tracetool.py main
entrypoint ?

I guess I'd be inclined to change the global initialization to just
be 'None' to make it explicit that out_open is expected to always be
called ?

> > +        out_fobj = sys.stdout
> > +    else:
> > +        out_filename = posix_relpath(filename)
> 
> I have CCed Oleg in case he spots any portability issues, but I think
> this should still work on Windows.

This use of posix_relpath() was pre-existing, so there shouldn't be
any new issues from this.

> 
> > +        out_fobj = open(filename, 'wt')
> >  
> >  def out(*lines, **kwargs):
> >      """Write a set of output lines.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|