Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6c20bdb

Browse files
committed
Further tweak memory management for regex DFAs.
Coverity is still unhappy after commit 190c798, and after looking closer I think it might be onto something. The callers of newdfa() typically drop out if v->err has been set nonzero, which newdfa() is faithfully doing if it fails. However, what if v->err was already nonzero before we entered newdfa()? Then newdfa() could succeed and the caller would promptly leak its result. I don't think this scenario can actually happen, but the predicate "v->err is always zero when newdfa() is called" seems difficult to be entirely sure of; there's a good deal of code that potentially could get that wrong. It seems better to adjust the callers to directly check for a null result instead of relying on ISERR() tests. This is slightly cheaper than the previous coding anyway. Lacking evidence that there's any real bug, no back-patch.
1 parent 8a812e5 commit 6c20bdb

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

src/backend/regex/rege_dfa.c

+2
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,8 @@ lastcold(struct vars *v,
604604

605605
/*
606606
* newdfa - set up a fresh DFA
607+
*
608+
* Returns NULL (and sets v->err) on failure.
607609
*/
608610
static struct dfa *
609611
newdfa(struct vars *v,

src/backend/regex/regexec.c

+8-10
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ getsubdfa(struct vars *v,
351351
if (d == NULL)
352352
{
353353
d = newdfa(v, &t->cnfa, &v->g->cmap, DOMALLOC);
354-
if (ISERR())
354+
if (d == NULL)
355355
return NULL;
356356
/* set up additional info if this is a backref node */
357357
if (t->op == 'b')
@@ -381,8 +381,6 @@ getladfa(struct vars *v,
381381
struct subre *sub = &v->g->lacons[n];
382382

383383
v->ladfas[n] = newdfa(v, &sub->cnfa, &v->g->cmap, DOMALLOC);
384-
if (ISERR())
385-
return NULL;
386384
/* a LACON can't contain a backref, so nothing else to do */
387385
}
388386
return v->ladfas[n];
@@ -408,8 +406,8 @@ find(struct vars *v,
408406

409407
/* first, a shot with the search RE */
410408
s = newdfa(v, &v->g->search, cm, &v->dfa1);
411-
assert(!(ISERR() && s != NULL));
412-
NOERR();
409+
if (s == NULL)
410+
return v->err;
413411
MDEBUG(("\nsearch at %ld\n", LOFF(v->start)));
414412
cold = NULL;
415413
close = shortest(v, s, v->search_start, v->search_start, v->stop,
@@ -436,8 +434,8 @@ find(struct vars *v,
436434
cold = NULL;
437435
MDEBUG(("between %ld and %ld\n", LOFF(open), LOFF(close)));
438436
d = newdfa(v, cnfa, cm, &v->dfa1);
439-
assert(!(ISERR() && d != NULL));
440-
NOERR();
437+
if (d == NULL)
438+
return v->err;
441439
for (begin = open; begin <= close; begin++)
442440
{
443441
MDEBUG(("\nfind trying at %ld\n", LOFF(begin)));
@@ -493,11 +491,11 @@ cfind(struct vars *v,
493491
int ret;
494492

495493
s = newdfa(v, &v->g->search, cm, &v->dfa1);
496-
NOERR();
494+
if (s == NULL)
495+
return v->err;
497496
d = newdfa(v, cnfa, cm, &v->dfa2);
498-
if (ISERR())
497+
if (d == NULL)
499498
{
500-
assert(d == NULL);
501499
freedfa(s);
502500
return v->err;
503501
}

0 commit comments

Comments
 (0)