X-Authentication-Warning: delorie.com: mailnull set sender to djgpp-workers-bounces using -f Sender: rich AT phekda DOT freeserve DOT co DOT uk Message-ID: <3C3B69C3.72383773@phekda.freeserve.co.uk> Date: Tue, 08 Jan 2002 21:50:59 +0000 From: Richard Dawe X-Mailer: Mozilla 4.77 [en] (X11; U; Linux 2.2.19 i586) X-Accept-Language: de,fr MIME-Version: 1.0 To: djgpp-workers AT delorie DOT com Subject: Re: Memory leaks fixes References: Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Reply-To: djgpp-workers AT delorie DOT com Hello! Eli Zaretskii wrote: > > On Sun, 6 Jan 2002, Richard Dawe wrote: > > > 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. No worries. About patch to redir_cmdline_parse: > > 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? I just looked at N843, a draft of the C99 standard that I downloaded off the web a couple of years ago. In section 7.20.3.4 "The realloc function" it says: "If the space cannot be allocated, the object pointed to by ptr is unchanged." So I think the patch works as I described. > > 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... Looking at it from a different angle: the function's description says: "Create a cmdline_t object, and return zero if all's well, non-zero otherwise." Well, if realloc() fails, then all is not well. So the current code is broken in that respect. Returning the original buffer (as the patch does) seems like the most graceful way of recovering from the failure IMHO. Of course, it still needs testing! Thanks, bye, Rich =] -- Richard Dawe [ http://www.phekda.freeserve.co.uk/richdawe/ ]