1. Patchew
  2. QEMU
  3. [PATCH RISU v2 00/13] ELF and Sparc64 support
  4. View patch

[PATCH RISU v2 05/13] risugen: Be explicit about print destinations

Richard Henderson posted 13 patches 6 months ago
[PATCH RISU v2 05/13] risugen: Be explicit about print destinations
Posted by Richard Henderson 6 months ago 
Printing directly to STDOUT and STDERR will allow the
print destination to be selected elsewhere.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 risugen_common.pm | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/risugen_common.pm b/risugen_common.pm
index 71ee996..5207c0e 100644
--- a/risugen_common.pm
+++ b/risugen_common.pm
@@ -76,7 +76,7 @@ sub progress_start($$)
     ($proglen, $progmax) = @_;
     $proglen -= 2; # allow for [] chars
     $| = 1;        # disable buffering so we can see the meter...
-    print "[" . " " x $proglen . "]\r";
+    print STDOUT "[" . " " x $proglen . "]\r";
     $lastprog = 0;
 }
 
@@ -87,13 +87,13 @@ sub progress_update($)
     my $barlen = int($proglen * $done / $progmax);
     if ($barlen != $lastprog) {
         $lastprog = $barlen;
-        print "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
+        print STDOUT "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
     }
 }
 
 sub progress_end()
 {
-    print "[" . "-" x $proglen . "]\n";
+    print STDOUT "[" . "-" x $proglen . "]\n";
     $| = 0;
 }
 
@@ -124,7 +124,7 @@ sub eval_with_fields($$$$$) {
     $evalstr .= "}";
     my $v = eval $evalstr;
     if ($@) {
-        print "Syntax error detected evaluating $insnname $blockname string:\n$block\n$@";
+        print STDERR "Syntax error detected evaluating $insnname $blockname string:\n$block\n$@";
         exit(1);
     }
     return $v;
@@ -163,20 +163,20 @@ sub dump_insn_details($$)
 {
     # Dump the instruction details for one insn
     my ($insn, $rec) = @_;
-    print "insn $insn: ";
+    print STDOUT "insn $insn: ";
     my $insnwidth = $rec->{width};
     my $fixedbits = $rec->{fixedbits};
     my $fixedbitmask = $rec->{fixedbitmask};
     my $constraint = $rec->{blocks}{"constraints"};
-    print sprintf(" insnwidth %d fixedbits %08x mask %08x ", $insnwidth, $fixedbits, $fixedbitmask);
+    print STDOUT sprintf(" insnwidth %d fixedbits %08x mask %08x ", $insnwidth, $fixedbits, $fixedbitmask);
     if (defined $constraint) {
-        print "constraint $constraint ";
+        print STDOUT "constraint $constraint ";
     }
     for my $tuple (@{ $rec->{fields} }) {
         my ($var, $pos, $mask) = @$tuple;
-        print "($var, $pos, " . sprintf("%08x", $mask) . ") ";
+        print STDOUT "($var, $pos, " . sprintf("%08x", $mask) . ") ";
     }
-    print "\n";
+    print STDOUT "\n";
 }
 
 1;
-- 
2.34.1
Re: [PATCH RISU v2 05/13] risugen: Be explicit about print destinations
Posted by Peter Maydell 5 months, 4 weeks ago 
On Sun, 26 May 2024 at 20:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Printing directly to STDOUT and STDERR will allow the
> print destination to be selected elsewhere.

i.e. using 'select' to set the default filehandle for "print"?
My instinct is to suspect that would be a bit confusing compared
to passing in an explicit filehandle for whatever it is that we'd
like to be able to print to other destinations.

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  risugen_common.pm | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/risugen_common.pm b/risugen_common.pm
> index 71ee996..5207c0e 100644
> --- a/risugen_common.pm
> +++ b/risugen_common.pm
> @@ -76,7 +76,7 @@ sub progress_start($$)
>      ($proglen, $progmax) = @_;
>      $proglen -= 2; # allow for [] chars
>      $| = 1;        # disable buffering so we can see the meter...
> -    print "[" . " " x $proglen . "]\r";
> +    print STDOUT "[" . " " x $proglen . "]\r";
>      $lastprog = 0;
>  }
>
> @@ -87,13 +87,13 @@ sub progress_update($)
>      my $barlen = int($proglen * $done / $progmax);
>      if ($barlen != $lastprog) {
>          $lastprog = $barlen;
> -        print "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
> +        print STDOUT "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
>      }
>  }
>
>  sub progress_end()
>  {
> -    print "[" . "-" x $proglen . "]\n";
> +    print STDOUT "[" . "-" x $proglen . "]\n";
>      $| = 0;
>  }

These are the progress-bar indicators -- shouldn't they go to STDERR,
not STDOUT, if we're going to be careful about where we send output?

> @@ -163,20 +163,20 @@ sub dump_insn_details($$)
>  {
>      # Dump the instruction details for one insn
>      my ($insn, $rec) = @_;
> -    print "insn $insn: ";
> +    print STDOUT "insn $insn: ";
>      my $insnwidth = $rec->{width};
>      my $fixedbits = $rec->{fixedbits};
>      my $fixedbitmask = $rec->{fixedbitmask};
>      my $constraint = $rec->{blocks}{"constraints"};
> -    print sprintf(" insnwidth %d fixedbits %08x mask %08x ", $insnwidth, $fixedbits, $fixedbitmask);
> +    print STDOUT sprintf(" insnwidth %d fixedbits %08x mask %08x ", $insnwidth, $fixedbits, $fixedbitmask);
>      if (defined $constraint) {
> -        print "constraint $constraint ";
> +        print STDOUT "constraint $constraint ";
>      }
>      for my $tuple (@{ $rec->{fields} }) {
>          my ($var, $pos, $mask) = @$tuple;
> -        print "($var, $pos, " . sprintf("%08x", $mask) . ") ";
> +        print STDOUT "($var, $pos, " . sprintf("%08x", $mask) . ") ";
>      }
> -    print "\n";
> +    print STDOUT "\n";
>  }

As a debug-print helper routine maybe this should be STDERR too?

thanks
-- PMM
Re: [PATCH RISU v2 05/13] risugen: Be explicit about print destinations
Posted by Richard Henderson 5 months, 4 weeks ago 
On 5/30/24 05:51, Peter Maydell wrote:
>> @@ -87,13 +87,13 @@ sub progress_update($)
>>       my $barlen = int($proglen * $done / $progmax);
>>       if ($barlen != $lastprog) {
>>           $lastprog = $barlen;
>> -        print "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
>> +        print STDOUT "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
>>       }
>>   }
>>
>>   sub progress_end()
>>   {
>> -    print "[" . "-" x $proglen . "]\n";
>> +    print STDOUT "[" . "-" x $proglen . "]\n";
>>       $| = 0;
>>   }
> 
> These are the progress-bar indicators -- shouldn't they go to STDERR,
> not STDOUT, if we're going to be careful about where we send output?

Why?  I would think that only errors would go do stderr...

> 
>> @@ -163,20 +163,20 @@ sub dump_insn_details($$)
>>   {
>>       # Dump the instruction details for one insn
>>       my ($insn, $rec) = @_;
>> -    print "insn $insn: ";
>> +    print STDOUT "insn $insn: ";
>>       my $insnwidth = $rec->{width};
>>       my $fixedbits = $rec->{fixedbits};
>>       my $fixedbitmask = $rec->{fixedbitmask};
>>       my $constraint = $rec->{blocks}{"constraints"};
>> -    print sprintf(" insnwidth %d fixedbits %08x mask %08x ", $insnwidth, $fixedbits, $fixedbitmask);
>> +    print STDOUT sprintf(" insnwidth %d fixedbits %08x mask %08x ", $insnwidth, $fixedbits, $fixedbitmask);
>>       if (defined $constraint) {
>> -        print "constraint $constraint ";
>> +        print STDOUT "constraint $constraint ";
>>       }
>>       for my $tuple (@{ $rec->{fields} }) {
>>           my ($var, $pos, $mask) = @$tuple;
>> -        print "($var, $pos, " . sprintf("%08x", $mask) . ") ";
>> +        print STDOUT "($var, $pos, " . sprintf("%08x", $mask) . ") ";
>>       }
>> -    print "\n";
>> +    print STDOUT "\n";
>>   }
> 
> As a debug-print helper routine maybe this should be STDERR too?

Likewise?


r~
Re: [PATCH RISU v2 05/13] risugen: Be explicit about print destinations
Posted by Peter Maydell 5 months, 3 weeks ago 
On Thu, 30 May 2024 at 18:37, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/30/24 05:51, Peter Maydell wrote:
> >> @@ -87,13 +87,13 @@ sub progress_update($)
> >>       my $barlen = int($proglen * $done / $progmax);
> >>       if ($barlen != $lastprog) {
> >>           $lastprog = $barlen;
> >> -        print "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
> >> +        print STDOUT "[" . "-" x $barlen . " " x ($proglen - $barlen) . "]\r";
> >>       }
> >>   }
> >>
> >>   sub progress_end()
> >>   {
> >> -    print "[" . "-" x $proglen . "]\n";
> >> +    print STDOUT "[" . "-" x $proglen . "]\n";
> >>       $| = 0;
> >>   }
> >
> > These are the progress-bar indicators -- shouldn't they go to STDERR,
> > not STDOUT, if we're going to be careful about where we send output?
>
> Why?  I would think that only errors would go do stderr...

Usually when programs have their stdout and stderr going to
different places it's because stderr is the terminal and
stdout is "wherever the 'normal output' of the program
should go". So progress bars and error messages go to stderr
because those are for the user reading the terminal, not
for whatever is consuming the normal output.

In the risugen case we always output to a named file,
so the distinction is less important.

thanks
-- PMM

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.