RFR(M): 8020775: PPC64 (part 12): posix signal printing

[Lindenmaier, Goetz]

Hi,

> I think it is good cleanup. Thank you, Goetz.
Thanks! Merits go to Thomas Stuefe who initially redesigned this.
I added him as contributor.

http://cr.openjdk.java.net/~goetz/webrevs/8020775-print_sig-2/

In general: The 'describe' functions compute the string containing
the desired information.  The 'print' functions decide how to 
publish this information.  It's just a separation of concern.  We even
test them separately.
We have a row of additional signal printing routines that are not
'short'.  We use them to write much more information into the hs_err
file.  I did not contribute them, as I assume you will not like it if the
hs_err file get's bigger.
In addition I added support for real time signals, and a workaround for redundant
SIGIOT.

> Could you align elements/names in tables in get_signal_name() and 
> get_signal_code_description() to make them aligned with each other and 
> with #ifdefs?
Done, see webrev.  It looks better for most of the tables, but not
for info[] in get_signal_name. I think it does not help there, I would prefer
the layout without all the white space.

> It looks like all unixes (including bsd) have NSIG value defined (I 
> could be wrong). you can use it directly instead of MAX_SIGNAL_NUMBER. 
There are some good reasons not to use NSIG for this constant, but 
they are in the code generating more verbose output, which I did not contribute.
So I removed it.  (E.g., sometimes it's a function and thus can not be used
as array size.)

> describe_signal_set_short() method is bogus. Why you overwrite buf[0] 
> with "0" or "1" and the rest of buffer with 0 each time? Should it be 
> And why you need separate describe_signal_set_short() method? And why 
> you need *10 for buffer size
Oh, you are right, fixed.  I had to change this, as we are using fixBufferStream for 
printing here.  We implemented fixBufferStream in ostream.hpp extending
outputStream to print to a given, fixed buffer.
I didn't want to contribute that because it's used only here in the port.
> I would prefer to see the code similar to 
> describe_sa_flags() with list of signals instead of "01". Yes, it would 
> be different from current code but it would much more useful to have 
> signal names.
It's the short routine, replacing an int printed in a line.  Signal names
would be far too long here.  I can contribute 
  STEP(225, "(printing signal handlers)" )
     if (_verbose) {
#ifdef _WIN32
       os::print_signal_handlers(st, buf, sizeof(buf));
#else
       os::Posix::print_current_signal_handlers(st); 
#endif
       st->cr();
     }
which would do what you have in mind here, I think.

> In describe_sa_flags():
> size_t remaining = size-1; // leave space for /0
I think this is correct.  Below we test accordingly.

> In get_signal_code_description(): Add break:
Fixed. 
> Put on different lines:
> +    out->s_name = out->s_desc = "unknown";
Fixed.

> Why not use s_name instead of s_code to match out->s_name?
I renamed this a bit, I hope this makes it better.

If you think it's useful for OpenJDK, I can contribute the fixBufferStream
and the more verbose printing methods.

Best regards,
  Goetz.

On 7/25/13 4:11 PM, Lindenmaier, Goetz wrote:
> Hi,
>
> we'd like to contribute our posix signal printing.
> We implemented some routines to print signal and sa_flags information
> in the os/posix files, and call them from
> os::print_siginfo and print_signal_handler() in the various unix
> variant directories.
> The output is a bit more verbose than the existing version.
>
> We contribute this here, as our aix code uses this too.
>
> Please review this and test it if you think we should add this.
> We'd appreciate it.
> http://cr.openjdk.java.net/~goetz/webrevs/8020775-print_sig/
>
> Thanks and best regards,
>    Goetz.
>