Re: Patch: fix various issues found by static analysis

[Patrice Dumas]

On Tue, Oct 15, 2024 at 09:34:08PM +0100, Gavin Smith wrote:
> On Tue, Oct 15, 2024 at 12:36:59PM +0200, Vitezslav Crhonek wrote:
> > From f9e62115a2ae91e721021f52bdf2c76fe717a5eb Mon Sep 17 00:00:00 2001
> > From: Vitezslav Crhonek <vcrhonek@redhat.com>
> > Date: Tue, 15 Oct 2024 11:07:06 +0200
> > Subject: [PATCH 3/7] * info/session.c: add initializer, allocate memory for
> >  the terminating null of the string
> > 
> > ---
> >  info/session.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/info/session.c b/info/session.c
> > index 660e8e477a..b0cd9bac18 100644
> > --- a/info/session.c
> > +++ b/info/session.c
> > @@ -2342,7 +2342,7 @@ info_menu_or_ref_item (WINDOW *window, int menu_item, 
> > int xref, int ask_p)
> >            if (defentry)
> >              {
> >                prompt = xmalloc (strlen (defentry->label)
> > -                                + strlen (_("Menu item (%s): ")));
> > +                                + strlen (_("Menu item (%s): ")) + 1);
> >                sprintf (prompt, _("Menu item (%s): "), defentry->label);
> >              }
> >            else
> > @@ -2353,7 +2353,7 @@ info_menu_or_ref_item (WINDOW *window, int menu_item, 
> > int xref, int ask_p)
> >            if (defentry)
> >              {
> >                prompt = xmalloc (strlen (defentry->label)
> > -                                + strlen (_("Follow xref (%s): ")));
> > +                                + strlen (_("Follow xref (%s): ")) + 1);
> >                sprintf (prompt, _("Follow xref (%s): "), defentry->label);
> >              }
> >            else
> > @@ -2949,7 +2949,7 @@ DECLARE_INFO_COMMAND (info_menu_sequence,
> >  static int
> >  info_handle_pointer (const char *label, WINDOW *window)
> >  {
> > -  char *description;
> > +  char *description = NULL;
> >    NODE *node;
> >  
> >    if (!strcmp (label, "Up"))
> > @@ -3508,7 +3508,7 @@ info_intuit_options_node (NODE *node, char *program)
> >          {
> >            char *nodename;
> >  
> > -          nodename = xmalloc (strlen (program) + strlen (*try_node));
> > +          nodename = xmalloc (strlen (program) + strlen (*try_node) + 1);
> >            sprintf (nodename, *try_node, program);
> >            /* The last resort "%s" is dangerous, so we restrict it
> >               to exact matches here.  */
> > @@ -3584,7 +3584,7 @@ DECLARE_INFO_COMMAND (info_goto_invocation_node,
> >    default_program_name = program_name_from_file_name (file_name);
> >  
> >    prompt = xmalloc (strlen (default_program_name) +
> > -               strlen (invocation_prompt));
> > +               strlen (invocation_prompt) + 1);
> >    sprintf (prompt, invocation_prompt, default_program_name);
> >    line = info_read_in_echo_area (prompt);
> >    free (prompt);
> 
> I used xasprintf (wrapper around libc function asprintf) here instead
> as it is shorter and more robust.

I agree that it is better with xasprintf, simpler and more readable, but
I also think that there was no need for the +1 in the initial code as
the %s in the format was removed from the final string, so this allowed
to have a \0 and even one more unused byte in memory.

-- 
Pat