Re: [PATCH] Install and generate docs for gdb-add-index

Sergio Durigan Junior
2018-01-12T04:18:00Z

On Thursday, January 11 2018, Simon Marchi wrote:

> On 2018-01-11 04:35 PM, Sergio Durigan Junior wrote:
>> The "gdb-add-index" script has been resurrected on:
>>
>> commit caf26be91a584ef141ac5d3cb31007731af8b8e3
>> Author: Samuel Bronson <naesten@gmail.com>
>> Date: Fri Nov 15 16:09:33 2013 -0500
>>
>> Resurrect gdb-add-index as a contrib script
>>
>> However, for some reason (I couldn't find it in the archives), only
>> the script has been checked-in; the Makefile parts responsible for
>> installing it in the system were left out. This commit fixes that, by
>> also resurrecting the Makefile and documentation bits.
>>
>> This commit is part of our effort to upstream the local Fedora GDB
>> changes. With this commit, we'll only carry a very small
>> Fedora-specific modification to the script.
>
> Woo thanks!
>
> I have some comments, but I'm just being picky.

Thanks for the review, Simon.

>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index 17b71c6e7c..d63220db4e 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -1770,6 +1770,15 @@ install-only: $(CONFIG_INSTALL)
>> $(INSTALL_SCRIPT) gcore \
>> $(DESTDIR)$(bindir)/$$transformed_name; \
>> fi
>> + transformed_name=`t='$(program_transform_name)'; \
>> + echo gdb-add-index | sed -e "$$t"` ; \
>> + if test "x$$transformed_name" = x; then \
>> + transformed_name=gdb-add-index ; \
>> + else \
>> + true ; \
>> + fi ; \
>> + $(INSTALL_PROGRAM) $(srcdir)/contrib/gdb-add-index.sh \
>> + $(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT)
>
> I know this is just copy pasted from above, but doesn't the indentation look odd?
> Shouldn't the if/else/fi and the INSTALL_PROGRAM lines be indented with one tab
> less?

Ah, you're right. Strange, I don't know why it is indented this way
originally. Anyway, fixed it.

>> @$(MAKE) DO=install "DODIRS=$(SUBDIRS)" $(FLAGS_TO_PASS) subdir_do
>>
>> install-strip:
>> diff --git a/gdb/doc/Makefile.in b/gdb/doc/Makefile.in
>> index eafadee899..0323bc4ecb 100644
>> --- a/gdb/doc/Makefile.in
>> +++ b/gdb/doc/Makefile.in
>> @@ -174,7 +174,7 @@ POD2MAN5 = pod2man --center="GNU Development Tools" \
>> --release="gdb-`sed q version.subst`" --section=5
>>
>> # List of man pages generated from gdb.texi
>> -MAN1S = gdb.1 gdbserver.1 gcore.1
>> +MAN1S = gdb.1 gdbserver.1 gcore.1 gdb-add-index.1
>> MAN5S = gdbinit.5
>> MANS = $(MAN1S) $(MAN5S)
>>
>> @@ -633,6 +633,13 @@ gcore.1: $(GDB_DOC_FILES)
>> mv -f $@.T$$$$ $@) || (rm -f $@.T$$$$ && exit 1)
>> rm -f gcore.pod
>>
>> +gdb-add-index.1: $(GDB_DOC_FILES)
>> + touch $@
>> + -$(TEXI2POD) $(MANCONF) -Dgdb-add-index < $(srcdir)/gdb.texinfo > gdb-add-index.pod
>> + -($(POD2MAN1) gdb-add-index.pod | sed -e '/^.if n .na/d' > $@.T$$$$ && \
>> + mv -f $@.T$$$$ $@) || (rm -f $@.T$$$$ && exit 1)
>> + rm -f gdb-add-index.pod
>> +
>> gdbinit.5: $(GDB_DOC_FILES)
>> touch $@
>> -$(TEXI2POD) $(MANCONF) -Dgdbinit < $(srcdir)/gdb.texinfo > gdbinit.pod
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index dba2fa766a..62fcdd524e 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -19775,6 +19775,14 @@ There are currently some limitation on indices. They only work when
>> for DWARF debugging information, not stabs. And, they do not
>> currently work for programs using Ada.
>>
>> +@value{GDBN} comes with a program, @command{gdb-add-index}, which can
>> +be used to add the index to a symbol file. It takes the symbol file
>> +as its only argument:
>> +
>> +@smallexample
>> +$ gdb-add-index symfile
>> +@end smallexample
>> +
>
> Would it be good to put the part about gdb-add-index earlier in the page/section,
> for example just after the first paragraph? The doc starts by telling you the
> manual method with the scary objcopy lines and all. I think it would be nice if
> it started by telling: here's the easy and recommended way, and then here are all
> the gory details about how it's done internally, if you need it. I could imagine
> users starting to read the section, and stopping midway because they think it's
> too complicated/not worth it, and never get to the gdb-add-index part.

That's a good point. I moved the text up (after the first paragraph, as
suggested) and modified it a bit:

...
@node Index Files
@section Index Files Speed Up @value{GDBN}
@cindex index files
@cindex @samp{.gdb_index} section

When @value{GDBN} finds a symbol file, it scans the symbols in the
file in order to construct an internal symbol table. This lets most
@value{GDBN} operations work quickly---at the cost of a delay early
on. For large programs, this delay can be quite lengthy, so
@value{GDBN} provides a way to build an index, which speeds up
startup.

For convenience, @value{GDBN} comes with a program,
@command{gdb-add-index}, which can be used to add the index to a
symbol file. It takes the symbol file as its only argument:

@smallexample
$ gdb-add-index symfile
@end smallexample

It is also possible to do the work manually. Here is what
@command{gdb-add-index} does behind the curtains.
...

I will submit the full patch as a v2.

Thanks,

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible