242 // These functions are similar to the above "Get" version with the significant 243 // difference that the preference module will alloc the memory (e.g. XP_STRDUP) and 244 // the caller will need to be responsible for freeing it... 245 // </font> 246 */ 247 PrefResult PREF_CopyCharPref(const char *pref, char ** return_buf, PRBool isDefault); 679 PREF_CopyCharPref(const char *pref_name, char ** return_buffer, PRBool get_default) 680 { ... 698 *return_buffer = PL_strdup(stringVal); .... 703 } * Referenced (in 3 files total) in: * modules/libpref/src/nsPrefBranch.cpp: o View change log or Blame annotations line 212 o line 834 o line 945 Is the only consumer (Bug 213691 covers mozilla's bad paste job) 943 NS_IMETHODIMP nsPrefBranch::SecurityGetCharPref(const char *pref, char ** return_buf) 944 { 945 return _convertRes(PREF_CopyCharPref(getPrefName(pref), return_buf, PR_FALSE)); 946 } Note that the caller is an xpcom method and its out param should use the xpcom allocator which is not compatible w/ pl_strdupn

While I was looking, I also found: * nsPrefService uses PR_Free on memory prefapi.cpp allocated with ToNewCString * prefapi.cpp uses PR_Free/PR_FREEIF on memory allocated with PL_strdup * prefapi.cpp uses PR_Free on memory allocated with malloc In this last case I could have changed the allocator instead; for the other cases there is no compatible allocator for that particular deallocator.

Comment on attachment 368353 [details] [diff] [review] Proposed patch as a reminder, PL_strfree while being the correct pair to PL_strdup, is not null safe :) Based on the way the code uses pointers, I don't think you can simply drop the null checks: 663 oldValue->stringVal = newValue.stringVal ? PL_strdup(newValue.stringVal) : NULL;

Strangely missing from the previous attachment was the hunk that was actually trying to fix the bug as described...

Pushed changeset 0ee18978858a to mozilla-central.