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)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.0

People

(Reporter: roland.mainz, Assigned: cathleennscp)

References

()

Details

(Keywords: perf)

Attachments

(1 file, 2 obsolete files)

[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);|
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);|
Status: NEW → ASSIGNED
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. <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 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+
fine...
Attachment #106761 - Attachment is obsolete: true
landed
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: