Re: [PATCH] V2 Begin adding some docs to elfutils

[Mark Wielaard]

Hi Ben,

On Fri, 2019-08-23 at 13:42 -0700, Ben Woodard wrote:
> - Added doc/README

I like this one. If only because it says we would like to move to
sphinx :) I must admit I am not a fan of raw nroff. But of course it is
better than nothing.

> - Updated doc/ChangeLog

Thanks. You don't really have to add new entries for changes between
different versions never committed. Feel free to just add entries for
the final result.

> - Added a eu-readelf manpage based on the one from binutils.

I still have some reservations about this, see below.

> - Added a brand new manpage for eu-elfclassify the new utility added
> in 0.177

This is nice. But maybe also a good example of what help2man could do
(of course we don't have the machinery to generate help2man pages for
now).

> - Add some new files in the doc directory and sync makefile with
> upstream.
> - Reenable the compilation of doc directory.
> - Disable sgml file building
> - Build man pages the automake way

I had to tweak this in 3 ways.

First when doing a make dist the pdf and dvi files would still be
generated. So I just removed the build rules completely.

Second "Man pages are not currently considered to be source, because it
is not uncommon for man pages to be automatically generated.  Therefore
they are not automatically included in the distribution." (*) So they
need to have a dist_ prefix to get actually included in the dist.

Third we use --program-prefix=eu- by default, which renames all our
binaries to eu-xxx (the user could use no or another prefix, although
that is discouraged, because it would clash with the binutil names).
This also applies to man-pages. So they get installed as eu-eu-
readelf.1 and eu-elf_begin.3. To fix this I renamed the .1 files to not
have the eu-prefix by default. And for the .3 function pages I added
the notrans_ prefix (*).

(*) See also 
https://www.gnu.org/software/automake/manual/html_node/Man-Pages.html

This is the diff that I came up with:

index cc0a470b..f11fb8f0 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -16,19 +16,6 @@
 ##
 ## You should have received a copy of the GNU General Public License
 ## along with this program.  If not, see .
-EXTRA_DIST = COPYING
-man1_MANS=eu-readelf.1 eu-elfclassify.1
-man3_MANS=elf_update.3 elf_getdata.3 elf_clone.3 elf_begin.3
-
-CLEANFILES = elfutils.dvi
-
-# We need only a few special rules to generate the various output formats
-# from the SGML sources.
-.PHONY: dvi pdf html
-pdf: $(srcdir)elfutils.pdf
-dvi: $(srcdir)elfutils.dvi
-
-$(srcdir)%.dvi: %.sgml
-   db2dvi $^
-$(srcdir)%.pdf: %.sgml
-   db2pdf $^
+EXTRA_DIST = COPYING README
+dist_man1_MANS=readelf.1 elfclassify.1
+notrans_dist_man3_MANS=elf_update.3 elf_getdata.3 elf_clone.3 elf_begin.3

Also for the .3 manpages I replace \— with \-. Since the former comes
out really odd on my (RHEL7) system.

> Since V1
> - Put man pages in the proper directories.
> - Added copy of Gnu Free Documentation License
> - Modified eu-readelf.1 man page to match the supported options.

There are still some flags/options that seem specific to binutils
readelf.

The short form for -a mentions --use-dynamic, which eu-readelf doesn't
know about (I removed the paragraph from the man page).

Our help also says that -x, --hex-dump, might take a section number or
a section name. But it doesn't actually work! The patch to fix that is
simple though. This is good, the documenting found an actual bug :)

diff --git a/src/readelf.c b/src/readelf.c
index 24be7a9a..77f9c3a3 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -12634,6 +12634,7 @@ for_each_section_argument (Elf *elf, const struct 
section_argument *list,
error (EXIT_FAILURE, 0, gettext ("cannot get section header: %s"),
   elf_errmsg (-1));
  name = elf_strptr (elf, shstrndx, shdr_mem.sh_name);
+ (*dump) (scn, &shdr_mem, name);
}
   else
{

The short form of --debug-dump, -w[lLiaprmfFsoRtUuTgAckK], doesn't
actually work for eu-readelf (maybe it should, but I removed it from
the man page since it currently doesn't).

eu-readelf also doesn't support --debug-dump=rawline (which seems to be
an alias for =line?), =frame-interp, =trace_info, =trace_abbrev,
=trace_aranges, =pubtype, =cu_index nor -links. All have been removed
from the man page.

Also we advertise --debug-dump=frame in --help, while the man page
advertises --debug-dump=frames. eu-readelf support both. binutils
readelf only support =frames). I didn't change the man page.

readelf --debug-dump=ranges seems to be an alias from --debug-
dump=aranges. eu-readelf uses --debug-dump=ranges for .debug_ranges (or
.debug_rnglist). binutils readelf uses =Ranges (with a capital R) for
the later. eu-readelf doesn't support =Ranges. Changed the man page to
use the lower case name.

--debug-dump=follow-links seems somewhat related to our --debug-
dump=info+. Removed =follow-links, added =info+.

eu-readelf implements -w, --wide, b

[PATCH] readelf: Actually dump hex or strings when -p or -x get section number.

[Mark Wielaard]

The readelf code did parse section numbers, but then failed to actually
dump the section found. Fixed by actually calling the dump function
(either the hex or string variant). Add testcase for readelf -x num.

Signed-off-by: Mark Wielaard 
---
 src/ChangeLog  | 5 +
 src/readelf.c  | 1 +
 tests/ChangeLog| 4 
 tests/run-readelf-test2.sh | 7 +++
 4 files changed, 17 insertions(+)

diff --git a/src/ChangeLog b/src/ChangeLog
index ad673c11..78a63e48 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-27  Mark Wielaard  
+
+   * readelf.c (for_each_section_argument): Call (*dump) also for
+   section numbers.
+
 2019-08-26  Mark Wielaard  
 
* readelf (options): Add OPTION_ARG_OPTIONAL "SECTION" for notes.
diff --git a/src/readelf.c b/src/readelf.c
index 24be7a9a..77f9c3a3 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -12634,6 +12634,7 @@ for_each_section_argument (Elf *elf, const struct 
section_argument *list,
error (EXIT_FAILURE, 0, gettext ("cannot get section header: %s"),
   elf_errmsg (-1));
  name = elf_strptr (elf, shstrndx, shdr_mem.sh_name);
+ (*dump) (scn, &shdr_mem, name);
}
   else
{
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 2900bd82..cb336d68 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,7 @@
+2019-08-27  Mark Wielaard  
+
+   * run-readelf-test2.sh: Add -x num testcase.
+
 2019-07-26  Florian Weimer  
Mark Wielaard  
 
diff --git a/tests/run-readelf-test2.sh b/tests/run-readelf-test2.sh
index 90306247..e03ee8d7 100755
--- a/tests/run-readelf-test2.sh
+++ b/tests/run-readelf-test2.sh
@@ -25,4 +25,11 @@ Hex dump of section [6] '.strtab', 1 bytes at offset 0x290:
   0x 00  .
 EOF
 
+testrun_compare ${abs_top_builddir}/src/readelf -x 6 testfile28 <<\EOF
+
+Hex dump of section [6] '.strtab', 1 bytes at offset 0x290:
+  0x 00  .
+EOF
+
+
 exit 0
-- 
2.18.1

[PATCH] config: Add manpages to spec file.

[Mark Wielaard]

Now that we have manpages lets also package them.

Signed-off-by: Mark Wielaard 
---
 config/ChangeLog| 5 +
 config/elfutils.spec.in | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/config/ChangeLog b/config/ChangeLog
index 12a141b2..f4d80224 100644
--- a/config/ChangeLog
+++ b/config/ChangeLog
@@ -1,3 +1,8 @@
+2019-08-28  Mark Wielaard  
+
+   * elfutils.spec.in (%files): Add man1/eu-*.1*.
+   (%files libelf-devel): Add man3/elf_*.3*.
+
 2019-08-14  Dmitry V. Levin  
 
* elfutils.spec.in (%files): Add %{_bindir}/eu-elfclassify.
diff --git a/config/elfutils.spec.in b/config/elfutils.spec.in
index c5896a41..3d8ec668 100644
--- a/config/elfutils.spec.in
+++ b/config/elfutils.spec.in
@@ -184,6 +184,7 @@ rm -rf ${RPM_BUILD_ROOT}
 %{_libdir}/libdw.so.*
 %dir %{_libdir}/elfutils
 %{_libdir}/elfutils/lib*.so
+%{_mandir}/man1/eu-*.1*
 
 %files devel
 %defattr(-,root,root)
@@ -220,6 +221,7 @@ rm -rf ${RPM_BUILD_ROOT}
 %{_includedir}/elfutils/version.h
 %{_libdir}/libelf.so
 %{_libdir}/pkgconfig/libelf.pc
+%{_mandir}/man3/elf_*.3*
 
 %files libelf-devel-static
 %{_libdir}/libelf.a
-- 
2.18.1