Re: Corrupted directory output

[Brendan O'Dea]

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 <gavinsmith0123@gmail.com> 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