Mail Archives: djgpp-workers/2002/01/06/06:13:55
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...
- Raw text -