X-Authentication-Warning: delorie.com: mailnull set sender to djgpp-workers-bounces using -f
Date: Sun, 6 Jan 2002 13:13:03 +0200 (IST)
From: Eli Zaretskii <eliz AT is DOT elta DOT co DOT il>
X-Sender: eliz AT is
To: Richard Dawe <rich AT phekda DOT freeserve DOT co DOT uk>
cc: djgpp-workers AT delorie DOT com
Subject: Re: Memory leaks fixes
In-Reply-To: <3C38331A.4D044C9A@phekda.freeserve.co.uk>
Message-ID: <Pine.SUN.3.91.1020106130422.2000A-100000@is>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Reply-To: djgpp-workers AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk


On Sun, 6 Jan 2002, Richard Dawe wrote:

> > Why do you only call realloc if fsext_list is NULL?  AFAICS, it
> > should be called unconditionally, since this code expands the table,
> > not only creates it from scratch.  Am I missing something?
> 
> With the patch grow_table() is as below. We do realloc() unconditionally.
> This may not have been clear from the diff.

Yes, I've misread the diffs, sorry.

> > > --- src/debug/common/dbgredir.c 1999/06/14 16:20:41     1.3
> > > +++ src/debug/common/dbgredir.c 2002/01/05 12:27:57
> > > @@ -452,8 +452,10 @@ redir_cmdline_parse (const char *args, c
> > >         }
> > >      }
> > >    *d = '\0';
> > > -  /* Free unused space.  */
> > > -  cmd->command = (char *) realloc (cmd->command, cmd_len + 1);
> > > +  /* Free unused space, if we can.  */
> > > +  d = (char *) realloc (cmd->command, cmd_len + 1);
> > > +  if (d)
> > > +    cmd->command = d;
> > 
> > No, please don't!  This code compacts the string, and thus the result
> > will always be _smaller_ than the original.  So if realloc fails here,
> > the right thing to do would be to return the original string.
> 
> Do you mean 'args' by 'the original string'?

No, I meant cmd->command.

> Surely it's
> better to return the cmd->command buffer we originally allocated, if we
> can't shrink it? (This is what the patch does.)

I'm not sure that's what the patch does: is it certain that realloc never 
touches the original buffer, even if it fails?

> It seems unlikely that this code will fail, due to memory exhaustion. But
> I am concerned that the return code of realloc() was not checked.

If we are sure realloc will never fail in this case, why do we need to 
check its return value?

Anyway, the reallocation was simply an attempt to be nice; the gains in 
memory are probably miniscule.  Perhaps it would be simpler to not 
realloc at all...