Simplify memory management for regex DFAs a little.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 22 Feb 2021 01:29:11 +0000 (20:29 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 22 Feb 2021 01:29:11 +0000 (20:29 -0500)
Coverity complained that functions in regexec.c might leak DFA
storage.  It's wrong, but this logic is confusing enough that it's
not so surprising Coverity couldn't make sense of it.  Rewrite
in hopes of making it more legible to humans as well as machines.

src/backend/regex/rege_dfa.c
src/backend/regex/regexec.c

index 957ceb8137de691e0cf7455ac96d7da0fde89970..1db52d1005c415b06c7556a24693111544baf5b5 100644 (file)
@@ -499,7 +499,7 @@ newdfa(struct vars *v,
        struct dfa *d;
        size_t          nss = cnfa->nstates * 2;
        int                     wordsper = (cnfa->nstates + UBITS - 1) / UBITS;
-       struct smalldfa *smallwas = sml;
+       bool            ismalloced = false;
 
        assert(cnfa != NULL && cnfa->nstates != 0);
 
@@ -514,6 +514,7 @@ newdfa(struct vars *v,
                                ERR(REG_ESPACE);
                                return NULL;
                        }
+                       ismalloced = true;
                }
                d = &sml->dfa;
                d->ssets = sml->ssets;
@@ -521,8 +522,8 @@ newdfa(struct vars *v,
                d->work = &d->statesarea[nss];
                d->outsarea = sml->outsarea;
                d->incarea = sml->incarea;
-               d->cptsmalloced = 0;
-               d->mallocarea = (smallwas == NULL) ? (char *) sml : NULL;
+               d->ismalloced = ismalloced;
+               d->arraysmalloced = false;      /* not separately allocated, anyway */
        }
        else
        {
@@ -540,8 +541,9 @@ newdfa(struct vars *v,
                                                                                          sizeof(struct sset *));
                d->incarea = (struct arcp *) MALLOC(nss * cnfa->ncolors *
                                                                                        sizeof(struct arcp));
-               d->cptsmalloced = 1;
-               d->mallocarea = (char *) d;
+               d->ismalloced = true;
+               d->arraysmalloced = true;
+               /* now freedfa() will behave sanely */
                if (d->ssets == NULL || d->statesarea == NULL ||
                        d->outsarea == NULL || d->incarea == NULL)
                {
@@ -573,7 +575,7 @@ newdfa(struct vars *v,
 static void
 freedfa(struct dfa *d)
 {
-       if (d->cptsmalloced)
+       if (d->arraysmalloced)
        {
                if (d->ssets != NULL)
                        FREE(d->ssets);
@@ -585,8 +587,8 @@ freedfa(struct dfa *d)
                        FREE(d->incarea);
        }
 
-       if (d->mallocarea != NULL)
-               FREE(d->mallocarea);
+       if (d->ismalloced)
+               FREE(d);
 }
 
 /*
index 2a1ef0176a50090b545dbc67b8263139a1382217..cf95989948964414b6c6069863011a8bc5b07c2c 100644 (file)
@@ -77,8 +77,8 @@ struct dfa
        chr                *lastpost;           /* location of last cache-flushed success */
        chr                *lastnopr;           /* location of last cache-flushed NOPROGRESS */
        struct sset *search;            /* replacement-search-pointer memory */
-       int                     cptsmalloced;   /* were the areas individually malloced? */
-       char       *mallocarea;         /* self, or master malloced area, or NULL */
+       bool            ismalloced;             /* should this struct dfa be freed? */
+       bool            arraysmalloced; /* should its subsidiary arrays be freed? */
 };
 
 #define WORK   1                               /* number of work bitvectors needed */
@@ -88,7 +88,7 @@ struct dfa
 #define FEWCOLORS      15
 struct smalldfa
 {
-       struct dfa      dfa;
+       struct dfa      dfa;                    /* must be first */
        struct sset ssets[FEWSTATES * 2];
        unsigned        statesarea[FEWSTATES * 2 + WORK];
        struct sset *outsarea[FEWSTATES * 2 * FEWCOLORS];