Change the way string relopts are allocated.
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 9 Aug 2011 12:25:44 +0000 (15:25 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 9 Aug 2011 12:25:44 +0000 (15:25 +0300)
Don't try to allocate the default value for a string relopt in the same
palloc chunk as the relopt_string struct. That didn't work too well if you
added a built-in string relopt in the stringRelOpts array, as it's not
possible to have an initializer for a variable length struct in C. This
makes the code slightly simpler too.

While we're at it, move the call to validator function in
add_string_reloption to before the allocation, so that if someone does pass
a bogus default value, we don't leak memory.

src/backend/access/common/reloptions.c
src/include/access/reloptions.h

index 465742556f5b990b9bff095e757802477c01d299..900b222865e15ae482d0ac46b8ed8a159275de23 100644 (file)
@@ -371,8 +371,6 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
        size_t          size;
        relopt_gen *newoption;
 
-       Assert(type != RELOPT_TYPE_STRING);
-
        oldcxt = MemoryContextSwitchTo(TopMemoryContext);
 
        switch (type)
@@ -386,6 +384,9 @@ allocate_reloption(bits32 kinds, int type, char *name, char *desc)
                case RELOPT_TYPE_REAL:
                        size = sizeof(relopt_real);
                        break;
+               case RELOPT_TYPE_STRING:
+                       size = sizeof(relopt_string);
+                       break;
                default:
                        elog(ERROR, "unsupported option type");
                        return NULL;            /* keep compiler quiet */
@@ -474,45 +475,29 @@ void
 add_string_reloption(bits32 kinds, char *name, char *desc, char *default_val,
                                         validate_string_relopt validator)
 {
-       MemoryContext oldcxt;
        relopt_string *newoption;
-       int                     default_len = 0;
-
-       oldcxt = MemoryContextSwitchTo(TopMemoryContext);
-
-       if (default_val)
-               default_len = strlen(default_val);
 
-       newoption = palloc0(sizeof(relopt_string) + default_len);
+       /* make sure the validator/default combination is sane */
+       if (validator)
+               (validator) (default_val);
 
-       newoption->gen.name = pstrdup(name);
-       if (desc)
-               newoption->gen.desc = pstrdup(desc);
-       else
-               newoption->gen.desc = NULL;
-       newoption->gen.kinds = kinds;
-       newoption->gen.namelen = strlen(name);
-       newoption->gen.type = RELOPT_TYPE_STRING;
+       newoption = (relopt_string *) allocate_reloption(kinds, RELOPT_TYPE_STRING,
+                                                                                                        name, desc);
        newoption->validate_cb = validator;
        if (default_val)
        {
-               strcpy(newoption->default_val, default_val);
-               newoption->default_len = default_len;
+               newoption->default_val = MemoryContextStrdup(TopMemoryContext,
+                                                                                                        default_val);
+               newoption->default_len = strlen(default_val);
                newoption->default_isnull = false;
        }
        else
        {
-               newoption->default_val[0] = '\0';
+               newoption->default_val = "";
                newoption->default_len = 0;
                newoption->default_isnull = true;
        }
 
-       /* make sure the validator/default combination is sane */
-       if (newoption->validate_cb)
-               (newoption->validate_cb) (newoption->default_val);
-
-       MemoryContextSwitchTo(oldcxt);
-
        add_reloption((relopt_gen *) newoption);
 }
 
index c7709cc058956f0cf416a310e8f81770f25e0d28..14f50345bbf4faf3f354bce3864edee33726a27e 100644 (file)
@@ -108,7 +108,7 @@ typedef struct relopt_string
        int                     default_len;
        bool            default_isnull;
        validate_string_relopt validate_cb;
-       char            default_val[1]; /* variable length, zero-terminated */
+       char       *default_val;
 } relopt_string;
 
 /* This is the table datatype for fillRelOptions */