Hi Housam,
On Thu, Sep 28, 2023 at 11:00 AM Housam Alamour via Elfutils-devel
<elfutils-devel@sourceware.org> wrote:
>
> Here is the new srcfiles tool ready for review.
Thanks for working on this.
I was not able to apply the patch from your email due to some line
breaks that your email client may have inserted. `git format-patch` and
`git send-email` are useful for emailing patches with the right format.
However I was able to test the patch using your try-pr30000 git branch.
> diff --git a/doc/srcfiles.1 b/doc/srcfiles.1
> new file mode 100644
> index 00000000..245c2d27
> --- /dev/null
> +++ b/doc/srcfiles.1
srcfile.1 does not install since it's missing from doc/Makefile.am.
> @@ -0,0 +1,125 @@
> +.\" Copyright 2023 Red Hat Inc.
> +.\" Mon 2023-Sept 23 Housam Alamour <halam...@redhat.com>
> +.\" Contact elfutils-devel@sourceware.org to correct errors or typos.
> +.TH EU-SRCFILES 1 "2023-Sept-25" "elfutils"
> +
> +.de SAMPLE
> +.br
> +.RS 0
> +.nf
> +.nh
> +\fB
> +..
> +.de ESAMPLE
> +\fP
> +.hy
> +.fi
> +.RE
> +..
> +
> +.SH "NAME"
> +eu-srcfiles \- Lists the source files of a dwarf/elf file.
DWARF and ELF are usually capitalized.
> +
> +.SH "SYNOPSIS"
> +eu-srcfiles [\fB\-0\fR|\fB\-\-null\fR] [\fB\-c\fR|\fB\-\-cu\-only\fR]
> [\fB\-v\fR|\fB\-\-verbose\fR] INPUT
This description of the arg format differs from `eu-srcfiles --help`
$ eu-srcfiles --help
Usage: eu-srcfiles [OPTION...] [FILE]
[...]
The --help description implies that `eu-srcfiles ./prog` should work
for some executable named prog but this is not the case.
The man page is correct that I have to include -e or --core for this
to work and that '-e a.out' is used as a default file name.
> +
> +.SH "DESCRIPTION"
> +\fBeu-srcfiles\fR lists the source files of a given \s-1dwarf/elf\s0
> +file. This list is based on a search of the dwarf debuginfo, which
> +may be automatically fetched by debuginfod if applicable. The target
> +file may be an executable, a coredump, a process, or even the running
> +kernel. The default is the file 'a.out'. The source file names are
> +made unique and printed to standard output.
> +
> +.SH "OPTIONS"
> +The long and short forms of options, shown here as alternatives, are
> +equivalent.
> +
> +.SS "Input Options"
> +
> +Only one INPUT option may be used. The default is '-e a.out'.
There are some input options, such as -k, that are listed in --help but
not included in the man page.
> diff --git a/src/srcfiles.cxx b/src/srcfiles.cxx
> new file mode 100644
> index 00000000..c948269d
> [...]
> +int
> +main (int argc, char *argv[])
> +{
> + int remaining;
> +
> + /* Parse and process arguments. This includes opening the modules. */
> + argp_children[0].argp = dwfl_standard_argp ();
> + argp_children[0].group = 1;
> +
> + Dwfl *dwfl = NULL;
> + (void) argp_parse (&argp, argc, argv, 0, &remaining, &dwfl);
> + assert (dwfl != NULL);
> + // Process all loaded modules - probably just one, except if -K or -p is
> used.
The use of /* */ and // for comments is inconsistent. Although to be
fair it's inconsistent in parts of elfutils too!
> diff --git a/tests/run-srcfiles-self.sh b/tests/run-srcfiles-self.sh
> new file mode 100755
> index 00000000..075ff7d2
> --- /dev/null
> +++ b/tests/run-srcfiles-self.sh
> @@ -0,0 +1,45 @@
> +#! /bin/sh
> +# Copyright (C) 2023 Red Hat, Inc.
> +# This file is part of elfutils.
> +#
> +# This file is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# elfutils is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +. $srcdir/test-subr.sh
> +
> +# Test different command line combinations on the srcfiles binary itself.
> +ET_EXEC="${abs_top_builddir}/src/srcfiles"
> +ET_PID=$$
> +
> +SRC_NAME="srcfiles.cxx"
> +
> +for null_arg in --null ""; do
> + for verbose_arg in --verbose ""; do
> + testrun $ET_EXEC $null_arg $verbose_arg -p $ET_PID > /dev/null
> +
> + # Ensure that the output contains srclines.cxx
> + cu_only=$(testrun $ET_EXEC $null_arg $verbose_arg -c -e $ET_EXEC)
> + default=$(testrun $ET_EXEC $null_arg $verbose_arg -e $ET_EXEC)
> + result1=$(echo "$cu_only" | grep "$SRC_NAME")
> + result2=$(echo "$default" | grep "$SRC_NAME")
> + if [ -z "$result1" ] || [-z "$result2"]; then
> + exit 1
> + fi
> +
> + # Ensure that the output with the cu-only option contains less source
> files
> + if [ "$cu_only" -gt "$default" ]; then
> + exit 1
> + fi
> + done
> +done
> +
These tests pass for me on F38 x86_64.
It looks like this only tests whether different command line options
generate the same output or not. Would it be possible to add a test
that checks whether a particular source file actually appears in the
output as intended? For example, you could test whether src/srcfiles.cxx
appears in the output of "$ET_EXEC -e $ET_EXEC".
Aaron
