Re: [PATCH v2] Add gstack script

[Keith Seitz <keiths@redhat.com>]

Hi,

On 12/12/24 11:36 PM, Eli Zaretskii wrote:

>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -62,6 +62,9 @@
>>   * Support for process record/replay and reverse debugging on loongarch*-linux*
>>     targets has been added.
>>   
>> +* Newly installed $prefix/bin/gstack uses GDB to print stack traces of
>> +  running processes.
> 
> This is okay, but please say that gstack is a Bash shell script.

Sure.

> Also, what did you want to say by the "newly installed" part?

Simply to point out that this is a new script which is installed.
How about:

* New bash script gstack which uses GDB to print stack traces of
   running processes.

>> +@node gstack man
>> +@heading gstack
>> +
>> +@c man title gstack Print a stack trace of a running program
>> +
>> +@format
>> +@c man begin SYNOPSIS gstack
>> +gstack [-h | --help] [-v | --version] @var{pid}
>> +@c man end
>> +@end format
>> +
>> +@c man begin DESCRIPTION gstack
>> +Print a stack trace of a running program with process ID @var{pid}.  If the process
>> +is multi-threaded, @command{gstack} outputs backtraces for every thread which exists
>> +in the process.
>> +@c man end
>> +
>> +@c man begin OPTIONS gstack
>> +@table @env
>> +@item --help
>> +@itemx -h
>> +List all options, with brief explanations.
>> +
>> +@item --version
>> +@itemx -v
>> +Print version information and then exit.
>> +@end table
>> +@c man end
>> +
>> +@c man begin SEEALSO gstack
>> +@ifset man
>> +The full documentation for @value{GDBN} is maintained as a Texinfo manual.
>> +If the @code{info} and @code{gdb} programs and @value{GDBN}'s Texinfo
>> +documentation are properly installed at your site, the command
>> +
>> +@smallexample
>> +info gdb
>> +@end smallexample
>> +
>> +@noindent
>> +should give you access to the complete manual.
>> +
>> +@cite{Using GDB: A Guide to the GNU Source-Level Debugger},
>> +Richard M. Stallman and Roland H. Pesch, July 1991.
>> +@end ifset
>> +@c man end
> 
> Is that all we want to tell in the manual about the script?  Even for
> a man page this is quite terse.  Can we expand on this some more?  At
> the very least I think we should tell that the script invokes G|DB to
> attach to the program and produce the backtraces, then detaches from
> the program.  Also, perhaps tell what happens if PID identifies a
> non-existent process?

You should see the manpage I rewrote. :-)

Nonetheless, your point is taken. I will elaborate a little bit more.
I've also dug into a few other GNU man pages and note that they
commonly list any environment variables influencing the behavior of
programs. I've completely forgotten to do that, so I will add those,
too.

>> +awk -- "$awk_script"
> 
> What if the system has only gawk or nawk or mawk?  Should the literal
> "awk" be a variable?

Oh, awk. This is definitely my bad. It was lost on me the difference
between building/installing GDB on a single machine and doing the
same on different machines. The environments are not exactly correct.
Since I live in a distro-centric world, these two cases reduce for me.

I've read through the documentation on AC_PROG_AWK, and I think it
best to follow the advice there, including "Limitations of Usual
Tools" bits on awk. I think there is one questionable construct in
my awk script.

I agree -- an AWK environment variable would be a valuable addition,
including borrowing some bits from AC_PROG_AWK to make sure we get
the preferred awk program.

Thank you for your prompt review! I'll send a v3 after awaiting any
additional feedback.

Keith