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

Status: RESOLVED FIXED

(SeaMonkey :: General, enhancement)

Description

[Roland Mainz]

[spin-off of bug 118135 ("change nsCRT::mem* to mem*") per
http://bugzilla.mozilla.org/show_bug.cgi?id=118135#c41]

RFE: Change all calls to |x=malloc(len); memset(x, 0, len);| to |x=calloc(len,
1);|

Comment 1

[Aaron Lehmann]

Attachment: replace malloc/memset with calloc

Here's a patch to fix the few remaining cases in the tree, except for stuff in
3rd party libraries.

Comment 2

[Aaron Lehmann]

Add self to CC

Comment 3

[timeless]

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.

Comment 4

[Aaron Lehmann]

Attachment: calloc v2

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.

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

sizeof(char) is defined as one by the C standard, so I think it's just noise.

But maybe that's just me.

Comment 6

[Simon Leinen]

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.  <[e-mail]>

Comment 7

[timeless]

Comment on attachment 106761 [details] [diff] [review]
calloc v2

that must have been a typo. i don't know how it happened. sorry!

Comment 8

[Boris Zbarsky [: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

Comment 9

[Aaron Lehmann]

Attachment: s/sizeof ?\((unsigned )?char\)/1/

fine...

Comment 10

[Aaron Lehmann]

landed