JS_ReportOutOfMemory() redundant

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
trivial
VERIFIED FIXED
17 years ago
14 years ago

People

(Reporter: Bernard Alleysson, Assigned: rogerl (gone))

Tracking

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

17 years ago
from jsregexp.c

static RENode *
374 NewRENode(CompilerState *state, REOp op, void *kid)
375 {
376 JSContext *cx;
377 RENode *ren;
378 379 cx = state->context;
380 ren = (RENode*) JS_malloc(cx, sizeof *ren);
381 if (!ren) {
382 JS_ReportOutOfMemory(cx);
383 return NULL;
384 }
385 ren->op = (uint8)op;
386 ren->flags = 0;
387 #ifdef DEBUG
388 ren->offset = gOffset++;
389 #endif
390 ren->next = NULL;
391 ren->kid = kid;
392 return ren;
393 }

the code
"
380 ren = (RENode*) JS_malloc(cx, sizeof *ren);
381 if (!ren) {
382 JS_ReportOutOfMemory(cx);
383 return NULL;
384 }
"

could be simplified as

"
ren = (RENode*) JS_malloc(cx, sizeof *ren);
if (!ren)
      return NULL;
"

because JS_ReportOutOfMemory() will be called by JS_malloc itself before return

all this in case of JS_malloc() failure wich is never likely to happen ... but
who knows :-)

Comment 1

17 years ago
Formally confirming bug for consideration. cc'ing Brendan in case he's 
interested in this one - 
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: JS_ReportOutOfMemory() redondant → JS_ReportOutOfMemory() redundant
I'll gladly r= a patch, but this is rogerl's code.

More significant, perhaps, is the number of small mallocs done to compile a
regexp.  It would be better to coalesce these somehow, using an arena perhaps,
and then copying (with pointer fixups) to the final malloc'd-to-tree-size piece
of memory.

/be

Comment 3

15 years ago
it looks like bug 85721's rewrite makes this obsolete.
Depends on: 85721
(Reporter)

Comment 4

14 years ago
yes, fixed by patch for bug 85721:
http://lxr.mozilla.org/seamonkey/source/js/src/jsregexp.c#275
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 5

14 years ago
Bernard: thanks for reporting back on this. Marking Verified -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.