Fixed formatting of man page and --help options and added additional test case to ensure the correct source file is output.

Message ID 20231024173528.682490-1-halamour@redhat.com
State Committed
Headers
Series Fixed formatting of man page and --help options and added additional test case to ensure the correct source file is output. |

Commit Message

Housam Alamour Oct. 24, 2023, 5:34 p.m. UTC
  Hello,
Here are the requested changes.
---
 doc/Makefile.am            |  2 +-
 doc/srcfiles.1             | 58 ++++++++++++++++++++++----------------
 src/srcfiles.cxx           | 29 ++++++++++---------
 tests/run-srcfiles-self.sh |  6 ++++
 4 files changed, 55 insertions(+), 40 deletions(-)
  

Comments

Aaron Merey Oct. 24, 2023, 7:59 p.m. UTC | #1
On Tue, Oct 24, 2023 at 1:35 PM Housam Alamour <halamour@redhat.com> wrote:
>
> Hello,
> Here are the requested changes.

Thanks Housam. I made a couple tweaks to the testcases, squashed this
patch into "PR 30000: debuginfod-find should have a source-list verb"
and merged it into the main branch.

Aaron
  

Patch

diff --git a/doc/Makefile.am b/doc/Makefile.am
index db5a842e..87de4f0b 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -17,7 +17,7 @@ 
 ## You should have received a copy of the GNU General Public License
 ## along with this program.  If not, see <http://www.gnu.org/licenses/>.
 EXTRA_DIST = COPYING-GFDL README
-dist_man1_MANS=readelf.1 elfclassify.1
+dist_man1_MANS=readelf.1 elfclassify.1 srcfiles.1
 notrans_dist_man3_MANS=elf_update.3 elf_getdata.3 elf_clone.3 elf_begin.3
 notrans_dist_man7_MANS=
 notrans_dist_man8_MANS=
diff --git a/doc/srcfiles.1 b/doc/srcfiles.1
index 245c2d27..da7c6989 100644
--- a/doc/srcfiles.1
+++ b/doc/srcfiles.1
@@ -18,55 +18,63 @@ 
 ..
 
 .SH "NAME"
-eu-srcfiles \- Lists the source files of a dwarf/elf file.
+eu-srcfiles \- Lists the source files of a DWARF/ELF file.
 
 .SH "SYNOPSIS"
 eu-srcfiles [\fB\-0\fR|\fB\-\-null\fR] [\fB\-c\fR|\fB\-\-cu\-only\fR] [\fB\-v\fR|\fB\-\-verbose\fR] INPUT
 
 .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
+\fBeu-srcfiles\fR lists the source files of a given \s-DWARF/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"
+.SH "INPUT OPTIONS"
 The long and short forms of options, shown here as alternatives, are
 equivalent.
+.TP
+\fB--core=COREFILE\fR
+Find addresses from signatures found in COREFILE.
+
+.TP
+\fB--debuginfo-path=PATH\fR
+Search path for separate debuginfo files.
 
-.SS "Input Options"
+.TP
+\fB-e FILE\fR, \fB--executable=FILE\fR
+Find addresses in FILE.
 
-Only one INPUT option may be used.  The default is '-e a.out'.
+.TP
+\fB-k\fR, \fB--kernel\fR
+Find addresses in the running kernel.
+
+.TP
+\fB-K\fR, \fB--offline-kernel[=RELEASE]\fR
+Kernel with all modules.
 
 .TP
-\fB\-e FILE\fR, \fB\-\-executable=FILE\fR
-Find source files for FILE.
+\fB-M FILE\fR, \fB--linux-process-map=FILE\fR
+Find addresses in files mapped as read from FILE in Linux /proc/PID/maps format.
 
 .TP
-\fB\-\-core=COREFILE\fR
-Find source files for the executable plus all shared libraries loaded
-into the coredumped process.
+\fB-p PID\fR, \fB--pid=PID\fR
+Find addresses in files mapped into process PID.
 
 .TP
-\fB\-p PID\fR, \fB\-\-pid=PID\fR
-Find source files for the executable plus all shared libraries loaded
-into running process PID.
+\fB-?\fR, \fB--help\fR
+Give this help list.
 
 .TP
-\fB\-M FILE\fR, \fB\-\-linux\-process\-map=FILE\fR
-Find source files for the executable plus all shared libraries loaded
-into a process, with given map file in linux /proc/PID/maps format.
-The file names listed in the map file must exist and be current on the
-system.
+\fB--usage\fR
+Give a short usage message.
 
 .TP
-\fB\-K, \-\-offline\-kernel[=RELEASE]
-Find source files for the kernel plus all installed modules
-with the given RELEASE version.  This may be very slow if
-dwarf is found via debuginfod.
+\fB-V\fR, \fB--version\fR
+Print program version.
 
-.SS "Output Options"
+.SH "OUTPUT OPTIONS"
 
 .TP
 \fB\-0, \-\-null\fR
@@ -78,7 +86,7 @@  Only list the CU names.
 
 .TP
 \fB\-v, \-\-verbose\fR
-Increase verbosity of logging messages to stderr.
+Increase verbosity of logging messages.
 
 
 .SH EXAMPLES
diff --git a/src/srcfiles.cxx b/src/srcfiles.cxx
index c948269d..a4cc91a1 100644
--- a/src/srcfiles.cxx
+++ b/src/srcfiles.cxx
@@ -41,8 +41,9 @@  ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
 /* Definitions of arguments for argp functions.  */
 static const struct argp_option options[] = 
 {
+  { NULL, 0, NULL, OPTION_DOC, N_("Output options:"), 1 },
   { "null", '0', NULL, 0,
-    N_ ("items are separated by a null, not whitespace."), 0 },
+    N_ ("Separate items by a null instead of a newline."), 0 },
   { "verbose", 'v', NULL, 0,
     N_ ("Increase verbosity of logging messages."), 0 },
   { "cu-only", 'c', NULL, 0, N_ ("Only list the CU names."), 0 },
@@ -50,10 +51,10 @@  static const struct argp_option options[] =
 };
 
 /* Short description of program.  */
-static const char doc[] = N_("Show source files of given ELF FILE.");
+static const char doc[] = N_("Lists the source files of a DWARF/ELF file. The default input is the file 'a.out'.");
 
 /* Strings for arguments in help texts.  */
-static const char args_doc[] = N_("[FILE]");
+static const char args_doc[] = N_("INPUT");
 
 /* Prototype for option handler.  */
 static error_t parse_opt (int key, char *arg, struct argp_state *state);
@@ -104,10 +105,10 @@  parse_opt (int key, char *arg, struct argp_state *state)
 }
 
 
-// Global list of collected source files.  Normally, it'll contain
-// the sources of just one named binary, but the '-K' option can cause
-// multiple dwfl modules to be loaded, thus listed.
-set<string> debug_sourcefiles;
+/* Global list of collected source files.  Normally, it'll contain
+   the sources of just one named binary, but the '-K' option can cause
+   multiple dwfl modules to be loaded, thus listed.   */
+   set<string> debug_sourcefiles;
 
 static int
 collect_sourcefiles (Dwfl_Module *dwflmod,
@@ -117,7 +118,7 @@  collect_sourcefiles (Dwfl_Module *dwflmod,
                      void *arg __attribute__ ((unused)))
 {
   Dwarf *dbg;
-  Dwarf_Addr bias; // ignored - for addressing purposes only
+  Dwarf_Addr bias; /* ignored - for addressing purposes only  */
   
   dbg = dwfl_module_getdwarf (dwflmod, &bias);
 
@@ -125,7 +126,7 @@  collect_sourcefiles (Dwfl_Module *dwflmod,
   Dwarf_Off old_offset;
   size_t hsize;
   
-  // Traverse all CUs of this module.
+  /* Traverse all CUs of this module.  */
   while (dwarf_nextcu (dbg, old_offset = offset, &offset, &hsize, NULL, NULL, NULL) == 0)
     {
       Dwarf_Die cudie_mem;
@@ -140,7 +141,7 @@  collect_sourcefiles (Dwfl_Module *dwflmod,
       if (dwarf_getsrcfiles (cudie, &files, &nfiles) != 0)
         continue;
 
-      // extract DW_AT_comp_dir to resolve relative file names
+      /* extract DW_AT_comp_dir to resolve relative file names  */
       const char *comp_dir = "";
       const char *const *dirs;
       size_t ndirs;
@@ -171,13 +172,13 @@  collect_sourcefiles (Dwfl_Module *dwflmod,
             continue;
 
           if (string(hat).find("<built-in>")
-              != std::string::npos) // gcc intrinsics, don't bother record
+              != std::string::npos) /* gcc intrinsics, don't bother record  */
             continue;
 
           string waldo;
-          if (hat[0] == '/') // absolute
+          if (hat[0] == '/') /* absolute */
             waldo = (string (hat));
-          else if (comp_dir[0] != '\0') // comp_dir relative
+          else if (comp_dir[0] != '\0') /* comp_dir relative */
             waldo = (string (comp_dir) + string ("/") + string (hat));
           debug_sourcefiles.insert (waldo);
         }
@@ -199,7 +200,7 @@  main (int argc, char *argv[])
   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.
+  /* Process all loaded modules - probably just one, except if -K or -p is used. */
   (void) dwfl_getmodules (dwfl, &collect_sourcefiles, NULL, 0);
 
   if (!debug_sourcefiles.empty ())
diff --git a/tests/run-srcfiles-self.sh b/tests/run-srcfiles-self.sh
index 075ff7d2..850a26d3 100755
--- a/tests/run-srcfiles-self.sh
+++ b/tests/run-srcfiles-self.sh
@@ -40,6 +40,12 @@  for null_arg in --null ""; do
     if [ "$cu_only" -gt "$default" ]; then
       exit 1
     fi
+
+    #Ensure the output contains the expected source file srcfiles.cxx
+    src=$(testrun $ET_EXEC -e $ET_EXEC)
+    if ! echo "$src" | grep -q "$SRC_NAME"; then
+      exit 1
+    fi
   done
 done