225898 - Is your realloc really necesary

Status: RESOLVED INCOMPLETE

(Core Graveyard :: GFX: Mac, defect)

Description

[Ben Fowler]

User-Agent:       Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 4.0)
Build Identifier: self built Camino version 0.7

I too am a reader of MacGuire's WCS, and whilst looking for the cause
of some wildly oversized memory requests in Thunderbird, I found that
nsMemory::Realloc( ) was being called with a null pointer.

Is this desirable?

Reproducible: Always

Steps to Reproduce:
1. Not relevant
2.
3.



Expected Results:  
I expect to be able to put a sanity check on memory internals of Mozilla

If you have read this far, you probably know that realloc when called
with a null pointer behaves like malloc.

Personally, I would prefer one of two symmetries:

A malloc( )balanced by free( )
B malloc( ) folloed by optional realloc( )s balanced by free( )

This requires the discipline to not knowingly pass a null pointer
to realloc, and gains the benefit of being able to assert on the 
value of pointer passed it.

I'm making this a severity of normal, but it is actually 'code quality'
less than normal.

Comment 1

[Boris Zbarsky [:bzbarsky]]

> I found that nsMemory::Realloc( ) was being called with a null pointer.

Called by whom?  This bug should be filed on the component doing that call.


Though I have to ask:

> B malloc( ) folloed by optional realloc( )s balanced by free( )

How is this substantively different if the malloc() is not called?

> and gains the benefit of being able to assert on the value of pointer passed
> it

Why is this a benefit?

Comment 2

[Jo Hermans]

Realloc() with a NULL pointer is valid behaviour according to the ANSI standard,
and should behave identically to malloc(). But not every implementation does
that, and it's always wise to avoid portability problems.

Comment 3

[Ben Fowler]

I was thinking of NS_IMETHODIMP nsRegionMac::GetRects(nsRegionRectSet **aRects)
in file nsRegionmac.cpp .

Did I mention that this seems to be called repeatedly whilst
Camino is idle?

Comment 4

[Ben Fowler]

I hesitate to take issue with ZB, but my point is that malloc( )/free( )
balances, and realloc( )/[realloc( )]* is also OK; I think that such schemata
are easier to check for correctness, more scrutable, than ones in which
realloc does the work of malloc, and one is trying to balance a free( )
with a preceding realloc( ).

I haven't come across a C system which gets this wrong, but I know many 
programmers (including myself once) who are unaware of the behviour of realloc( ).

Now that I do know, I never exploit its nuances ...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Over to the right component.

Ben, of you can produce a patch that would expedite things greatly... I'm not
aware of anyone actively working on Mac GFX.

Comment 6

[Ben Fowler]

I'm travelling to London for the Mac Expo shortly, I will certainly produce 
a patch and test it, but it may be a few weeks before I would be happy
to submit it to scrutiny!