On 03.04.2013 18:41, Tom Lane wrote:
Heikki Linnakangas<[email protected]> writes:
On 03.04.2013 18:21, Tom Lane wrote:
Obviously Henry didn't think that far ahead. I agree that throwing
an error is the best solution, and that widening "color" is probably
not what we want to do. You want to fix that, or shall I?
I can do it. I assume that Tcl has the same bug, so I'll submit a report
there, too.
Yes, definitely.
It occurs to me that at some point it might be useful to convert "color"
to unsigned short, so that you could have 64K of them ...--- but we'd still
need the error check anyway, and there's no reason to tackle such a
change today.
I was just thinking the same. In practice, expanding it to 64k doesn't
get you much farther. There is this in newdfa():
d->incarea = (struct arcp *) MALLOC(nss * cnfa->ncolors *
sizeof(struct arcp));
That's (number of states) * (number of colors) * (constant). The test
case I posted earlier would require about 40 GB of RAM for that
allocation alone, and fails with an "out of memory" error. Maybe it
would be possible to construct a regexp that has a lot of colors but few
states, but that's an even more marginal use case.
Attached is a patch to add the overflow check. I used the error message
"too many distinct characters in regex". That's not totally accurate,
because there isn't a limit on distinct characters per se, but on the
number of colors. Conceivably, you could have a regexp with more than
32k different characters, but where most of them are mapped to the same
color. In practice, it's not helpful to the user to say "too many
colors"; he will have no clue what a color is.
PS. I was mistaken when I said that this causes an assertion failure; it
segfaults even with assertions enabled.
- Heikki
diff --git a/src/backend/regex/regc_color.c b/src/backend/regex/regc_color.c
index 1c60566..e6aa899 100644
--- a/src/backend/regex/regc_color.c
+++ b/src/backend/regex/regc_color.c
@@ -247,7 +247,15 @@ newcolor(struct colormap * cm)
/* oops, must allocate more */
struct colordesc *newCd;
+ if (cm->max == MAX_COLOR)
+ {
+ CERR(REG_ECOLORS);
+ return COLORLESS; /* too many colors */
+ }
+
n = cm->ncds * 2;
+ if (n > MAX_COLOR + 1)
+ n = MAX_COLOR + 1;
if (cm->cd == cm->cdspace)
{
newCd = (struct colordesc *) MALLOC(n * sizeof(struct colordesc));
diff --git a/src/include/regex/regerrs.h b/src/include/regex/regerrs.h
index a761371..6516d7e 100644
--- a/src/include/regex/regerrs.h
+++ b/src/include/regex/regerrs.h
@@ -77,3 +77,7 @@
{
REG_ETOOBIG, "REG_ETOOBIG", "nfa has too many states"
},
+
+{
+ REG_ECOLORS, "REG_ECOLORS", "too many distinct characters in regex"
+},
diff --git a/src/include/regex/regex.h b/src/include/regex/regex.h
index 616c2c6..a7fb14f 100644
--- a/src/include/regex/regex.h
+++ b/src/include/regex/regex.h
@@ -153,6 +153,7 @@ typedef struct
#define REG_MIXED 17 /* character widths of regex and string differ */
#define REG_BADOPT 18 /* invalid embedded option */
#define REG_ETOOBIG 19 /* nfa has too many states */
+#define REG_ECOLORS 20 /* too many distinct characters in regex */
/* two specials for debugging and testing */
#define REG_ATOI 101 /* convert error-code name to number */
#define REG_ITOA 102 /* convert error-code number to name */
diff --git a/src/include/regex/regguts.h b/src/include/regex/regguts.h
index e1e406f..fc7f5ae 100644
--- a/src/include/regex/regguts.h
+++ b/src/include/regex/regguts.h
@@ -148,6 +148,7 @@
typedef short color; /* colors of characters */
typedef int pcolor; /* what color promotes to */
+#define MAX_COLOR 32767 /* max value that fits in 'color' datatype */
#define COLORLESS (-1) /* impossible color */
#define WHITE 0 /* default color, parent of all others */
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
