Thanks Gavin, that looks good. valgrind no longer complains about the folding.info input, and the output doesn't include garbage.
I've sent a change to the folding-mode maintainer to fix their index: https://github.com/jaalto/project-emacs--folding-mode/pull/9 A couple of other minor nits I noticed when looking into this that you may want to apply for the next release: - The function extract_menu_file_name is unused, consider removing it. - One instance of struct spec_entry is not correctly initialised (patch is attached). That doesn't cause problems now as far as I can tell, but doesn't hurt to fix. Thanks again for your quick fix. --bod On Sun, 21 Nov 2021 at 09:00, Gavin Smith <[email protected]> wrote: > On Sun, Nov 21, 2021 at 01:40:13AM +1100, Brendan O'Dea wrote: > > Malformed info files cause install-info to produce corrupted dir files. > > > > See attached example, where the folding info page contains: > > > > START-INFO-DIR-ENTRY > > * folding: folding editor minor mode for Emacs > > END-INFO-DIR-ENTRY > > > > which results in the corrupted `dir` file in the attached archive. > > > > I'm submitting a patch to the folding author to fix the @direntry for > that > > package, but install-info should probably be more resilient to badly > formed > > input. > > > > Some poking around suggests that the problem is in reformat_new_entries, > > where this sequence near the top of the for loop: > > > > split_entry (entry->text, &name, &name_len, &desc, &desc_len); > > free (entry->text); > > > > frees entry->text. When format_entry is called at the bottom that loop, > it > > exits early due to this condition (desc is NULL): > > > > if (!desc || !name) > > return 1; > > > > ...which does not set outstr_out (entry->text), leading to the > corruption. > > > > --bod > > Thanks for tracking it down to this. I'm not very familiar with the code > for install-info at all so it is slow for me to fix. (I'm not familiar > with all the options for install-info either.) > > The best I've been able to come up with at the moment is the following: > > diff --git a/install-info/install-info.c b/install-info/install-info.c > index e10f492261..32868acbf5 100644 > --- a/install-info/install-info.c > +++ b/install-info/install-info.c > @@ -1542,6 +1542,7 @@ format_entry (char *name, size_t name_len, char > *desc, size_t desc_len, > if (offset_out) > strncat (outstr, line_out, offset_out); > > + free (*outstr_out); > *outstr_out = outstr; > *outstr_len = strlen (outstr); > return 1; > @@ -1657,7 +1658,6 @@ reformat_new_entries (struct spec_entry *entries, > int calign_cli, int align_cli, > char *name = NULL, *desc = NULL; > size_t name_len = 0, desc_len = 0; > split_entry (entry->text, &name, &name_len, &desc, &desc_len); > - free (entry->text); > > /* Specify sane defaults if we need to */ > if (calign_cli == -1 || align_cli == -1) > > This works okay (tested). Do you think it is all right? I'll commit this > code soon if nobody has any other ideas. > > I couldn't see an easy way in reformat_new_entries to remove the > faulty entry. I'd prefer to fix this in the simplest, least intrusive > way possible so as not to risk breaking something. > > I had tried adding error code to set the line to an empty string if > format_entry was going to exit early, but this led to problems later > on in the code (in menu_line_equal) which didn't work for empty strings. > This meant that existing (correct) lines in the dir file would be deleted > due to menu_line_equal incorrectly returning true. Rather than try to fix > this, just pass through the original line unaltered. With your test this > leads to the final output: > > --- > File: dir, Node: Top This is the top of the INFO tree > > This (the Directory node) gives a menu of major topics. > Typing "q" exits, "H" lists all Info commands, "d" returns here, > "h" gives a primer for first-timers, > "mEmacs<Return>" visits the Emacs manual, etc. > > In Emacs, you can click mouse button 2 on a menu item or cross reference > to select it. > > * Menu: > > Emacs > * Emacs FAQ: (efaq). Frequently Asked Questions about Emacs. > * folding: folding editor minor mode for Emacs > --- > > and I have checked this is idempotent for the input files you sent (running > install-info more than once on the same file doesn't make any more > changes). > > > >
diff
Description: Binary data
