94570 - JS_ReportOutOfMemory() redundant

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bernard Alleysson]

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

[Phil Schwartau]

Formally confirming bug for consideration. cc'ing Brendan in case he's 
interested in this one - 

Comment 2

[Brendan Eich [:brendan]]

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

[timeless]

it looks like bug 85721's rewrite makes this obsolete.

Comment 4

[Bernard Alleysson]

yes, fixed by patch for bug 85721:
http://lxr.mozilla.org/seamonkey/source/js/src/jsregexp.c#275

Comment 5

[Phil Schwartau]

Bernard: thanks for reporting back on this. Marking Verified -