Closed
Bug 124302
Opened 24 years ago
Closed 23 years ago
Change |x=malloc(len); memset(x, 0, len);| to |x=calloc(1, len);|
Categories
(SeaMonkey :: General, enhancement)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.0
People
(Reporter: roland.mainz, Assigned: cathleennscp)
References
()
Details
(Keywords: perf)
Attachments
(1 file, 2 obsolete files)
12.75 KB,
patch
|
Details | Diff | Splinter Review |
[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);|
Reporter | ||
Updated•24 years ago
|
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);|
Updated•23 years ago
|
Keywords: mozilla1.0+
Comment 1•23 years ago
|
||
Here's a patch to fix the few remaining cases in the tree, except for stuff in
3rd party libraries.
Comment 2•23 years ago
|
||
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+
Comment 4•23 years ago
|
||
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
Comment 5•23 years ago
|
||
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)
Comment 6•23 years ago
|
||
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. <simon@switch.ch>
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 8•23 years ago
|
||
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+
Comment 10•23 years ago
|
||
landed
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Updated•21 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•