The distinction between moz_malloc/moz_free and malloc/free is not interesting. We are inconsistent in our use of one or the other, and I wouldn't be surprised if we are mixing them anyways. One difficulty is that things withing mozglue itself can't really use malloc/free verbatim because of how it's linked. See bug 868814 comment 10.

Didn't we make malloc() infallible?

Infallible is moz_xmalloc.

The problem from bug 868814 comment 10 still exists, but is more or less controlled with the changes in mozalloc.h. It won't prevent someone using malloc somehow in mozglue.dll, but I don't want to block this on waiting for a complete solution. And those should be easy to spot, as mozglue is a rather small surface, especially on Windows.

Comment on attachment 8583708 [details] [diff] [review] Remove moz_malloc/moz_free/moz_realloc/moz_calloc Review of attachment 8583708 [details] [diff] [review]: ----------------------------------------------------------------- If you haven't done so already, please grep for any remaining mentions of the removed moz_ functions. ::: memory/mozalloc/mozalloc.h @@ +67,5 @@ > + * Each declaration below is analogous to a "standard" allocation > + * function, except that the out-of-memory handling is made explicit. > + * The |moz_x| versions will never return a NULL pointer; if memory > + * is exhausted, they abort. The |moz_| versions may return NULL > + * pointers if memory is exhausted: their return value must be checked. The second half of this comment should be modified or removed, because the moz_ versions no longer exist. Maybe change it to this: "Each declaration below is analogous to a "standard" allocation function, except that it will never return a NULL pointer; if memory is exhausted, it will abort."

(In reply to Nicholas Nethercote [:njn] from comment #5) > Comment on attachment 8583708 [details] [diff] [review] > Remove moz_malloc/moz_free/moz_realloc/moz_calloc > > Review of attachment 8583708 [details] [diff] [review]: > ----------------------------------------------------------------- > > If you haven't done so already, please grep for any remaining mentions of > the removed moz_ functions. Did this every time I rebased this patch, and surprisingly, nothing piled up. > ::: memory/mozalloc/mozalloc.h > @@ +67,5 @@ > > + * Each declaration below is analogous to a "standard" allocation > > + * function, except that the out-of-memory handling is made explicit. > > + * The |moz_x| versions will never return a NULL pointer; if memory > > + * is exhausted, they abort. The |moz_| versions may return NULL > > + * pointers if memory is exhausted: their return value must be checked. > > The second half of this comment should be modified or removed, because the > moz_ versions no longer exist. Maybe change it to this: Actually, I left it on purpose, because there /still/ are a few moz_ functions.

Comment on attachment 8583707 [details] [diff] [review] Use malloc/free/realloc/calloc instead of moz_malloc/moz_free/moz_realloc/moz_calloc Review of attachment 8583707 [details] [diff] [review]: ----------------------------------------------------------------- Please do a full try server run before landing these patches :) ::: dom/base/nsTextFragment.cpp @@ +348,5 @@ > if (first16bit != -1) { // aBuffer contains no non-8bit character > // The old data was 1-byte, but the new is not so we have to expand it > // all to 2-byte > + char16_t* buff = (char16_t*)malloc((mState.mLength + aLength) * > + sizeof(char16_t)); I'd put the linebreak after the '=' and get the rest on a single line. @@ +383,1 @@ > (mState.mLength + aLength) * sizeof(char)); Adjust indentation of second line. ::: dom/media/webrtc/AudioOutputObserver.h @@ +51,5 @@ > nsAutoPtr<webrtc::SingleRwFifo> mPlayoutFifo; > uint32_t mChunkSize; > > // chunking to 10ms support > + FarEndAudioChunk *mSaved; // can't be nsAutoPtr since we need to use free() Pre-existing, but can you add ", not delete" to the comment? Because I had to look at the definition of nsAutoPtr to understand this comment. ::: gfx/graphite2/src/MozGrMalloc.h @@ +5,5 @@ > > #ifndef MOZ_GR_MALLOC_H > #define MOZ_GR_MALLOC_H > > +// Override malloc() and friends to call moz_xmalloc() etc, so that we get Good catch! ::: gfx/skia/trunk/src/ports/SkMemory_mozalloc.cpp @@ +39,1 @@ > } It's weird that there aren't equivalent versions of this function for calloc and realloc. Hmm. ::: intl/uconv/nsScriptableUConv.cpp @@ +150,5 @@ > rv = mDecoder->GetMaxLength(reinterpret_cast<const char*>(aData), > inLength, &outLength); > if (NS_SUCCEEDED(rv)) > { > + char16_t* buf = (char16_t*)malloc((outLength+1)*sizeof(char16_t)); Might as well add whitespace around the '*' while you're here.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5d54a547bdc https://hg.mozilla.org/integration/mozilla-inbound/rev/f2c533ee4071

I know at least those need an update: https://developer.mozilla.org/en-US/docs/Infallible_memory_allocation https://developer.mozilla.org/en-US/docs/Choosing_the_right_memory_allocator But maybe they should be consolidated instead. Also, there is going to be more changes (like bug 1134920 and bug 1134923) that will need doc updates as well. I expect those to be landed during this cycle (40). Maybe it's worth waiting for documentation updates. Please contact me to discuss this.

https://hg.mozilla.org/mozilla-central/rev/b5d54a547bdc https://hg.mozilla.org/mozilla-central/rev/f2c533ee4071

A reviewer cited MDN as a reason for me to use moz_malloc -- which I couldn't do. Can you please update these docs: https://developer.mozilla.org/en-US/docs/Infallible_memory_allocation https://developer.mozilla.org/en-US/docs/Choosing_the_right_memory_allocator https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIMemory and possibly others. In addition to changing the function names, I think we need to re-visit the prose as well. For example, is |malloc| still expected to become "eventually fallible"?

rather, "eventually infallible"

(In reply to David Major [:dmajor] from comment #11) > Can you please update these docs: > https://developer.mozilla.org/en-US/docs/Infallible_memory_allocation > https://developer.mozilla.org/en-US/docs/Choosing_the_right_memory_allocator > https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/ > Interface/nsIMemory > and possibly others. See comment 9.

MDN looks fine now