diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2021-03-08 16:32:29 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2021-03-08 16:32:29 -0500 |
commit | 6c20bdb2a279086777a3595ab00bcf14671fc5a1 (patch) | |
tree | e3072396155102c9b7b8c8c30962dc5cc2f4d565 /src/backend/regex/regexec.c | |
parent | 8a812e5106c5db50039336288d376a188844e2cc (diff) | |
download | postgresql-6c20bdb2a279086777a3595ab00bcf14671fc5a1.tar.gz postgresql-6c20bdb2a279086777a3595ab00bcf14671fc5a1.zip |
Further tweak memory management for regex DFAs.
Coverity is still unhappy after commit 190c79884, 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.
Diffstat (limited to 'src/backend/regex/regexec.c')
-rw-r--r-- | src/backend/regex/regexec.c | 18 |
1 files changed, 8 insertions, 10 deletions
diff --git a/src/backend/regex/regexec.c b/src/backend/regex/regexec.c index 8d7777f8c62..5b9a0878203 100644 --- a/src/backend/regex/regexec.c +++ b/src/backend/regex/regexec.c @@ -351,7 +351,7 @@ getsubdfa(struct vars *v, if (d == NULL) { d = newdfa(v, &t->cnfa, &v->g->cmap, DOMALLOC); - if (ISERR()) + if (d == NULL) return NULL; /* set up additional info if this is a backref node */ if (t->op == 'b') @@ -381,8 +381,6 @@ getladfa(struct vars *v, struct subre *sub = &v->g->lacons[n]; v->ladfas[n] = newdfa(v, &sub->cnfa, &v->g->cmap, DOMALLOC); - if (ISERR()) - return NULL; /* a LACON can't contain a backref, so nothing else to do */ } return v->ladfas[n]; @@ -408,8 +406,8 @@ find(struct vars *v, /* first, a shot with the search RE */ s = newdfa(v, &v->g->search, cm, &v->dfa1); - assert(!(ISERR() && s != NULL)); - NOERR(); + if (s == NULL) + return v->err; MDEBUG(("\nsearch at %ld\n", LOFF(v->start))); cold = NULL; close = shortest(v, s, v->search_start, v->search_start, v->stop, @@ -436,8 +434,8 @@ find(struct vars *v, cold = NULL; MDEBUG(("between %ld and %ld\n", LOFF(open), LOFF(close))); d = newdfa(v, cnfa, cm, &v->dfa1); - assert(!(ISERR() && d != NULL)); - NOERR(); + if (d == NULL) + return v->err; for (begin = open; begin <= close; begin++) { MDEBUG(("\nfind trying at %ld\n", LOFF(begin))); @@ -493,11 +491,11 @@ cfind(struct vars *v, int ret; s = newdfa(v, &v->g->search, cm, &v->dfa1); - NOERR(); + if (s == NULL) + return v->err; d = newdfa(v, cnfa, cm, &v->dfa2); - if (ISERR()) + if (d == NULL) { - assert(d == NULL); freedfa(s); return v->err; } |