103947 - unsafe realloc usage allows memory leak

Status: VERIFIED DUPLICATE of bug 103870

(MailNews Core :: Backend, defect)

Description

[Geoff Gustafson]

In compose/src/nsMsgSendLater.cpp, the BuildNewBuffer() function:

249   PRInt32 leftoverSize = PL_strlen(mLeftoverBuffer);
250   mLeftoverBuffer = (char *)PR_Realloc(mLeftoverBuffer, aCount + 
leftoverSize);
251   if (!mLeftoverBuffer)
252     return NS_ERROR_FAILURE;

This usage of realloc: a = realloc(a, size) is a common bug (described in the 
book Writing Solid Code).

If realloc returns null, the original pointer is overwritten and lost, hence a 
memory leak. Instead, it should be something like:

char *result = (char *)PR_Realloc(mLeftoverBuffer, aCount + leftoverSize);
if (result)
  mLeftoverBuffer = result;
else return NS_ERROR_FAILURE;

Or, as I believe Steve Maguire suggests in the book, you could wrap realloc 
with a function that separates the error condition from the result, e.g.

PRBool PR_Realloc(char **aBuffer, PRUint32 aCount);

It would be great to do that in NSPR itself as shown, to prevent the bug 
throughout all of Mozilla.

Comment 1

[Geoff Gustafson]

sorry, accidentally reposted form in a browser window open since this morning.

this bug is already present as 103870


*** This bug has been marked as a duplicate of 103870 ***

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

verified dup.