Closed Bug 124302 Opened 21 years ago Closed 21 years ago

Change |x=malloc(len); memset(x, 0, len);| to |x=calloc(1, len);|


(SeaMonkey :: General, enhancement)

Not set


(Not tracked)



(Reporter: roland.mainz, Assigned: cathleennscp)




(Keywords: perf)


(1 file, 2 obsolete files)

[spin-off of bug 118135 ("change nsCRT::mem* to mem*") per]

RFE: Change all calls to |x=malloc(len); memset(x, 0, len);| to |x=calloc(len,
Summary: Change |x=malloc(len); memset(x, 0, len);| to |x=calloc(len,1);| → Change |x=malloc(len); memset(x, 0, len);| to |x=calloc(1, len);|
Target Milestone: --- → mozilla0.9.9
Blocks: 71668
Keywords: perf
Keywords: mozilla1.0+
Target Milestone: mozilla0.9.9 → mozilla1.0
Depends on: 163472
Depends on: 163478
Here's a patch to fix the few remaining cases in the tree, except for stuff in
3rd party libraries.
Add self to CC
Comment on attachment 106644 [details] [diff] [review]
replace malloc/memset with calloc

>-	pEntry = (char_list *)malloc(sizeof(char_list));
>-	memset(pEntry, 0, sizeof(char_list));
>+	pEntry = (char_list *)calloc(sizeof(char_list), 1);
swap the params :) <- this is the only real comment

>-  handle->colorcube_d = malloc(sizeof(unsigned char) * 512);
>-  memset(handle->colorcube_d, 0, (sizeof(unsigned char) * 512));
>+  handle->colorcube_d = calloc(512, sizeof(unsigned char));
you're not consistent about 1 v. sizeof([unsigned] char), i think the sizeof()
syntax is nice (no magic numbers).

if it's possible to kill freeMem I think we'd probably appreciate it, but you
can do that in another bug/patch if you like.
Attachment #106644 - Flags: superreview?(bzbarsky)
Attachment #106644 - Flags: review+
Attached patch calloc v2 (obsolete) — Splinter Review
This addresses both comments, by swapping the arguments in one call where the
order is "wrong", and replacing the magic constant 1 with the equivalent
sizeof(char) in places where strings are being allocated.
Attachment #106644 - Attachment is obsolete: true
sizeof(char) is defined as one by the C standard, so I think it's just noise.

But maybe that's just me.
Attachment #106761 - Flags: superreview?(simon)
Attachment #106761 - Flags: review+
Attachment #106644 - Flags: superreview?(bzbarsky)
I have reviewed the changes and they look good.

However, shouldn't the return value of calloc() always be checked for NULL-ness
before the new objects are copied into?
Simon.  <>
Comment on attachment 106761 [details] [diff] [review]
calloc v2

that must have been a typo. i don't know how it happened. sorry!
Attachment #106761 - Flags: superreview?(simon) → superreview?(bzbarsky)
Comment on attachment 106761 [details] [diff] [review]
calloc v2

+    pEntry->m_pMacro = (char *)calloc(iLength + 1, sizeof (char));

space before the (char)?  why?

+		 WCHAR *pszNewURL = (WCHAR *) calloc(cbNewURL, 1);

Why not move the sizeof(WCHAR) out of the calculation of cbNewURL and into the
calloc()? Does anyone use cbNewURL after this point?

+  char* nextQuery = calloc(200,sizeof(char));

space before the sizeof()?

I agree with shaver.  sizeof(char) == 1, so let's type less.

With those changes, sr=me
Attachment #106761 - Flags: superreview?(bzbarsky) → superreview+
Attachment #106761 - Attachment is obsolete: true
Closed: 21 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.