public inbox for [email protected]help / color / mirror / Atom feed
bug in GUC 10+ messages / 4 participants [nested] [flat]
* bug in GUC @ 2004-06-24 05:27 Alvaro Herrera <[email protected]> 0 siblings, 2 replies; 10+ messages in thread From: Alvaro Herrera @ 2004-06-24 05:27 UTC (permalink / raw) To: Hackers <[email protected]> Hackers, I think there a bug in the GUC mechanism. The custom variables patch added several malloc() and a strdup() call, and they are never checked for an out of memory condition. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "El que vive para el futuro es un iluso, y el que vive para el pasado, un imbécil" (Luis Adler, "Los tripulantes de la noche") ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: bug in GUC @ 2004-06-24 06:49 Thomas Hallgren <[email protected]> parent: Alvaro Herrera <[email protected]> 1 sibling, 0 replies; 10+ messages in thread From: Thomas Hallgren @ 2004-06-24 06:49 UTC (permalink / raw) To: [email protected] I'll look into that. Thanks for pointing it out. Kind regards, Thomas Hallgren "Alvaro Herrera" <[email protected]> wrote in message news:[email protected]... > Hackers, > > I think there a bug in the GUC mechanism. The custom variables patch > added several malloc() and a strdup() call, and they are never checked > for an out of memory condition. > > -- > Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) > "El que vive para el futuro es un iluso, y el que vive para el pasado, > un imbécil" (Luis Adler, "Los tripulantes de la noche") > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org > ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: bug in GUC @ 2004-06-24 09:06 Thomas Hallgren <[email protected]> parent: Alvaro Herrera <[email protected]> 1 sibling, 1 reply; 10+ messages in thread From: Thomas Hallgren @ 2004-06-24 09:06 UTC (permalink / raw) To: [email protected] Rather than clutter the code with the same ereport over and over again (I count 12 malloc's in guc.c alone), I'd like something like this: void* malloc_or_fail(int elevel, size_t sz) { void* result; if(sz < 1) /* * Make sure we have something that can be passed to free() even * when the size is zero. */ sz = 1; result = malloc(sz); if(result == NULL) { ereport(elevel, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); /* Oops, ereport returned! Who called us with elevel < ERROR? */ exit(EXIT_FAILURE); } return result; } void* malloc_or_error(size_t sz) { return malloc_or_fail(ERROR, sz); } void* malloc_or_fatal(size_t sz) { return malloc_or_fail(FATAL, sz); } I search the code but the only thing I find that comes close is pq_malloc. But that's a different thing altogether since it doesn't use ereport. I'm sure I missed something somewhere but if not, perhaps the above functions would make sense? If so, what's the best name for them and where should they reside? Kind regards, Thomas Hallgren "Alvaro Herrera" <[email protected]> wrote in message news:[email protected]... > Hackers, > > I think there a bug in the GUC mechanism. The custom variables patch > added several malloc() and a strdup() call, and they are never checked > for an out of memory condition. > > -- > Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) > "El que vive para el futuro es un iluso, y el que vive para el pasado, > un imbécil" (Luis Adler, "Los tripulantes de la noche") > > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org > ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: bug in GUC @ 2004-06-24 14:02 Tom Lane <[email protected]> parent: Thomas Hallgren <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Tom Lane @ 2004-06-24 14:02 UTC (permalink / raw) To: Thomas Hallgren <[email protected]>; +Cc: [email protected] "Thomas Hallgren" <[email protected]> writes: > Rather than clutter the code with the same ereport over and over again (I > count 12 malloc's in guc.c alone), I'd like something like this: The larger question is why it contains even one. In general, use of malloc in the backend is the mark of a newbie. I'd think palloc in TopMemoryContext would be a more suitable approach. regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: bug in GUC @ 2004-06-24 14:45 Thomas Hallgren <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 2 replies; 10+ messages in thread From: Thomas Hallgren @ 2004-06-24 14:45 UTC (permalink / raw) To: [email protected] Ok, so I'm a newbie. To my defence I'll say that I made an effort to follow the style previously used in guc.c. The unchecked mallocs I added where not the first ;-) So, what you are saying is that there's no need for the functions I suggested and that a palloc using the TopMemoryContext will guarantee correct behavior on "out of memory"? Kind regards, Thomas Hallgren "Tom Lane" <[email protected]> wrote in message news:[email protected]... > "Thomas Hallgren" <[email protected]> writes: > > Rather than clutter the code with the same ereport over and over again (I > > count 12 malloc's in guc.c alone), I'd like something like this: > > The larger question is why it contains even one. In general, use of > malloc in the backend is the mark of a newbie. I'd think palloc in > TopMemoryContext would be a more suitable approach. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org > ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [HACKERS] bug in GUC @ 2004-06-24 15:59 James Robinson <[email protected]> parent: Thomas Hallgren <[email protected]> 1 sibling, 0 replies; 10+ messages in thread From: James Robinson @ 2004-06-24 15:59 UTC (permalink / raw) To: Thomas Hallgren <[email protected]>; +Cc: PostgreSQL Development <[email protected]>; pgsql-docs On Jun 24, 2004, at 10:45 AM, Thomas Hallgren wrote: > So, what you are saying is that there's no need for the functions I > suggested and that a palloc using the TopMemoryContext will guarantee > correct behavior on "out of memory"? Perhaps a section regarding proper memory management code in the backend could be written , say, somewhere in the internals document around the coding conventions chapter: http://developer.postgresql.org/docs/postgres/source.html I myself don't have a clue, not being a backend hacker, so I'll just slink back to my cave. ---- James Robinson Socialserve.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: bug in GUC @ 2004-06-24 16:55 Alvaro Herrera <[email protected]> parent: Thomas Hallgren <[email protected]> 1 sibling, 1 reply; 10+ messages in thread From: Alvaro Herrera @ 2004-06-24 16:55 UTC (permalink / raw) To: Thomas Hallgren <[email protected]>; +Cc: [email protected] On Thu, Jun 24, 2004 at 04:45:31PM +0200, Thomas Hallgren wrote: > Ok, so I'm a newbie. To my defence I'll say that I made an effort to follow > the style previously used in guc.c. The unchecked mallocs I added were not > the first ;-) Apparently Peter thought it was a good idea *not* to use palloc and friends, and documented it. The rationale seems to be "we have more control over out-of-memory conditions", and if you look closely, the out-of-memory is handled at a lower level than ERROR if it's not processing interactively. For example, when reading the config file, the ereport is DEBUG2. I'm not sure exactly why this is a good idea. After all, if the systems runs out of memory while starting up, what can be expected later? Not a lot is going to work. > So, what you are saying is that there's no need for the functions I > suggested and that a palloc using the TopMemoryContext will guarantee > correct behavior on "out of memory"? IMO yes and yes. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La felicidad no es mañana. La felicidad es ahora" ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: bug in GUC @ 2004-06-24 17:29 Tom Lane <[email protected]> parent: Alvaro Herrera <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Tom Lane @ 2004-06-24 17:29 UTC (permalink / raw) To: Alvaro Herrera <[email protected]>; +Cc: Thomas Hallgren <[email protected]>; [email protected] Alvaro Herrera <[email protected]> writes: > I'm not sure exactly why this is a good idea. After all, if the systems > runs out of memory while starting up, what can be expected later? The issue isn't with startup, but with re-reading postgresql.conf due to SIGHUP later on. We don't want to elog(ERROR) partway through that process. Especially not in the postmaster, where elog(ERROR) is tantamount to elog(FATAL). (But of course the postmaster shouldn't ever run out of memory anyway...) It's possible that this should all be rethought, but it would be a much more wide-ranging change than we've been discussing. regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [HACKERS] bug in GUC @ 2004-06-27 18:45 Thomas Hallgren <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Thomas Hallgren @ 2004-06-27 18:45 UTC (permalink / raw) To: Tom Lane <[email protected]> Tom Lane wrote: > Alvaro Herrera <[email protected]> writes: > > ... We don't want to elog(ERROR) partway through that > process. Especially not in the postmaster, where elog(ERROR) is > tantamount to elog(FATAL). (But of course the postmaster shouldn't ever > run out of memory anyway...) > > It's possible that this should all be rethought, but it would be a much > more wide-ranging change than we've been discussing. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 7: don't forget to increase your free space map settings > Here's a patch dealing with the unchecked mallocs and strdups in guc.c. Rather than mixing in palloc and TopMemoryContext into the code (would be rather messy given that most allocs actually falls into the category of not always being permitted to do elog(ERROR/FATAL)), I added a couple of simple static functions for guc_alloc, guc_realloc, and guc_strdup that are used throughout the guc.c file. Kind regards, Thomas Hallgren Index: src/backend/utils/misc/guc.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v retrieving revision 1.211 diff -u -r1.211 guc.c --- src/backend/utils/misc/guc.c 11 Jun 2004 03:54:54 -0000 1.211 +++ src/backend/utils/misc/guc.c 27 Jun 2004 18:43:18 -0000 @@ -1786,6 +1786,32 @@ static void ReportGUCOption(struct config_generic * record); static char *_ShowOption(struct config_generic * record); +static void* check_alloc(int elevel, void* data) +{ + if(data == NULL) + { + ereport(elevel, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + } + return data; +} + +static void* guc_alloc(int elevel, size_t size) +{ + return check_alloc(elevel, malloc(size)); +} + +static void* guc_realloc(int elevel, void* old, size_t size) +{ + return check_alloc(elevel, realloc(old, size)); +} + +static char* guc_strdup(int elevel, const char* src) +{ + return (char*)check_alloc(elevel, strdup(src)); +} + struct config_generic** get_guc_variables() { return guc_variables; @@ -1842,11 +1868,7 @@ size_vars = num_vars + num_vars / 4; guc_vars = (struct config_generic **) - malloc(size_vars * sizeof(struct config_generic *)); - if (!guc_vars) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + guc_alloc(FATAL, size_vars * sizeof(struct config_generic *)); num_vars = 0; @@ -1905,35 +1927,41 @@ * Add a new GUC variable to the list of known variables. The * list is expanded if needed. */ -static void -add_guc_variable(struct config_generic *var) +static bool +add_guc_variable(int elevel, struct config_generic *var) { if(num_guc_variables + 1 >= size_guc_variables) { /* Increase the vector with 20% */ - int size_vars = size_guc_variables + size_guc_variables / 4; + int size_vars = size_guc_variables + size_guc_variables / 5; struct config_generic** guc_vars; if(size_vars == 0) + { size_vars = 100; - - guc_vars = (struct config_generic**) - malloc(size_vars * sizeof(struct config_generic*)); - - if (guc_variables != NULL) + guc_vars = (struct config_generic**) + guc_alloc(elevel, size_vars * sizeof(struct config_generic*)); + } + else { - memcpy(guc_vars, guc_variables, - num_guc_variables * sizeof(struct config_generic*)); - free(guc_variables); + guc_vars = (struct config_generic**) + guc_realloc(elevel, guc_variables, size_vars * sizeof(struct config_generic*)); } - guc_variables = guc_vars; + if(guc_vars == NULL) + /* + * Out of memory + */ + return false; + size_guc_variables = size_vars; + guc_variables = guc_vars; } guc_variables[num_guc_variables++] = var; qsort((void*) guc_variables, num_guc_variables, sizeof(struct config_generic*), guc_var_compare); + return true; } /* @@ -1941,15 +1969,22 @@ * to a valid custom variable class at this point. */ static struct config_string* -add_placeholder_variable(const char *name) +add_placeholder_variable(int elevel, const char *name) { size_t sz = sizeof(struct config_string) + sizeof(char*); - struct config_string* var = (struct config_string*)malloc(sz); - struct config_generic* gen = &var->gen; + struct config_generic* gen; + + struct config_string* var = (struct config_string*)guc_alloc(elevel, sz); + if(var == NULL) + return NULL; + gen = &var->gen; memset(var, 0, sz); - gen->name = strdup(name); + gen->name = guc_strdup(elevel, name); + if(gen->name == NULL) + return NULL; + gen->context = PGC_USERSET; gen->group = CUSTOM_OPTIONS; gen->short_desc = "GUC placeholder variable"; @@ -1960,7 +1995,8 @@ * no 'static' place to point to. */ var->variable = (char**)(var + 1); - add_guc_variable((struct config_generic*)var); + if(!add_guc_variable(elevel, (struct config_generic*)var)) + var = NULL; return var; } @@ -1969,7 +2005,7 @@ * else return NULL. */ static struct config_generic * -find_option(const char *name) +find_option(int elevel, const char *name) { const char *dot; const char **key = &name; @@ -1998,7 +2034,7 @@ for (i = 0; map_old_guc_names[i] != NULL; i += 2) { if (guc_name_compare(name, map_old_guc_names[i]) == 0) - return find_option(map_old_guc_names[i+1]); + return find_option(elevel, map_old_guc_names[i+1]); } /* Check if the name is qualified, and if so, check if the qualifier @@ -2009,7 +2045,7 @@ /* * Add a placeholder variable for this name */ - return (struct config_generic*)add_placeholder_variable(name); + return (struct config_generic*)add_placeholder_variable(elevel, name); /* Unknown name */ return NULL; @@ -2159,11 +2195,7 @@ break; } - str = strdup(conf->boot_val); - if (str == NULL) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + str = guc_strdup(FATAL, conf->boot_val); conf->reset_val = str; if (conf->assign_hook) @@ -2710,7 +2742,7 @@ else elevel = ERROR; - record = find_option(name); + record = find_option(elevel, name); if (record == NULL) { ereport(elevel, @@ -3142,14 +3174,9 @@ if (value) { - newval = strdup(value); + newval = guc_strdup(elevel, value); if (newval == NULL) - { - ereport(elevel, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); return false; - } if (record->context == PGC_USERLIMIT) { @@ -3200,14 +3227,9 @@ * make this case work the same as the normal * assignment case. */ - newval = strdup(conf->reset_val); + newval = guc_strdup(elevel, conf->reset_val); if (newval == NULL) - { - ereport(elevel, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); return false; - } source = conf->gen.reset_source; } else @@ -3338,7 +3360,7 @@ struct config_generic *record; static char buffer[256]; - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -3374,7 +3396,7 @@ struct config_generic *record; static char buffer[256]; - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -3430,7 +3452,7 @@ return NULL; /* Else get flags for the variable */ - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -3601,7 +3623,7 @@ if(res == NULL) { - add_guc_variable(variable); + add_guc_variable(ERROR, variable); return; } @@ -3658,7 +3680,7 @@ GucContext context, enum config_type type) { - gen->name = strdup(name); + gen->name = guc_strdup(ERROR, name); gen->context = context; gen->group = CUSTOM_OPTIONS; gen->short_desc = short_desc; @@ -3676,7 +3698,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_bool); - struct config_bool* var = (struct config_bool*)malloc(sz); + struct config_bool* var = (struct config_bool*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_BOOL); @@ -3698,7 +3720,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_int); - struct config_int* var = (struct config_int*)malloc(sz); + struct config_int* var = (struct config_int*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_INT); @@ -3720,7 +3742,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_real); - struct config_real* var = (struct config_real*)malloc(sz); + struct config_real* var = (struct config_real*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_REAL); @@ -3742,7 +3764,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_string); - struct config_string* var = (struct config_string*)malloc(sz); + struct config_string* var = (struct config_string*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_STRING); @@ -3914,7 +3936,7 @@ { struct config_generic *record; - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -4281,16 +4303,15 @@ /* * Open file */ - new_filename = malloc(strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + + new_filename = guc_alloc(elevel, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + strlen(".new") + 2); - filename = malloc(strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); - if (new_filename == NULL || filename == NULL) - { - ereport(elevel, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + if(new_filename == NULL) return; - } + + filename = guc_alloc(elevel, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); + if (filename == NULL) + return; + sprintf(new_filename, "%s/" CONFIG_EXEC_PARAMS ".new", DataDir); sprintf(filename, "%s/" CONFIG_EXEC_PARAMS, DataDir); @@ -4402,9 +4423,9 @@ elog(FATAL, "invalid format of exec config params file"); } if (i == 0) - str = malloc(maxlen); + str = guc_alloc(FATAL, maxlen); else if (i == maxlen) - str = realloc(str, maxlen *= 2); + str = guc_realloc(FATAL, str, maxlen *= 2); str[i++] = ch; } while (ch != 0); @@ -4430,14 +4451,9 @@ /* * Open file */ - filename = malloc(strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); + filename = guc_alloc(ERROR, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); if (filename == NULL) - { - ereport(ERROR, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); return; - } sprintf(filename, "%s/" CONFIG_EXEC_PARAMS, DataDir); fp = AllocateFile(filename, "r"); @@ -4459,7 +4475,7 @@ if ((varname = read_string_with_null(fp)) == NULL) break; - if ((record = find_option(varname)) == NULL) + if ((record = find_option(FATAL, varname)) == NULL) elog(FATAL, "failed to locate variable %s in exec config params file",varname); if ((varvalue = read_string_with_null(fp)) == NULL) elog(FATAL, "invalid format of exec config params file"); @@ -4500,28 +4516,16 @@ if (string[equal_pos] == '=') { - *name = malloc(equal_pos + 1); - if (!*name) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + *name = guc_alloc(FATAL, equal_pos + 1); strncpy(*name, string, equal_pos); (*name)[equal_pos] = '\0'; - *value = strdup(&string[equal_pos + 1]); - if (!*value) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + *value = guc_strdup(FATAL, &string[equal_pos + 1]); } else { /* no equal sign in string */ - *name = strdup(string); - if (!*name) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + *name = guc_strdup(FATAL, string); *value = NULL; } Attachments: [text/plain] patch.txt (10.9K, 2-patch.txt) download | inline: Index: src/backend/utils/misc/guc.c =================================================================== RCS file: /projects/cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v retrieving revision 1.211 diff -u -r1.211 guc.c --- src/backend/utils/misc/guc.c 11 Jun 2004 03:54:54 -0000 1.211 +++ src/backend/utils/misc/guc.c 27 Jun 2004 18:43:18 -0000 @@ -1786,6 +1786,32 @@ static void ReportGUCOption(struct config_generic * record); static char *_ShowOption(struct config_generic * record); +static void* check_alloc(int elevel, void* data) +{ + if(data == NULL) + { + ereport(elevel, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); + } + return data; +} + +static void* guc_alloc(int elevel, size_t size) +{ + return check_alloc(elevel, malloc(size)); +} + +static void* guc_realloc(int elevel, void* old, size_t size) +{ + return check_alloc(elevel, realloc(old, size)); +} + +static char* guc_strdup(int elevel, const char* src) +{ + return (char*)check_alloc(elevel, strdup(src)); +} + struct config_generic** get_guc_variables() { return guc_variables; @@ -1842,11 +1868,7 @@ size_vars = num_vars + num_vars / 4; guc_vars = (struct config_generic **) - malloc(size_vars * sizeof(struct config_generic *)); - if (!guc_vars) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + guc_alloc(FATAL, size_vars * sizeof(struct config_generic *)); num_vars = 0; @@ -1905,35 +1927,41 @@ * Add a new GUC variable to the list of known variables. The * list is expanded if needed. */ -static void -add_guc_variable(struct config_generic *var) +static bool +add_guc_variable(int elevel, struct config_generic *var) { if(num_guc_variables + 1 >= size_guc_variables) { /* Increase the vector with 20% */ - int size_vars = size_guc_variables + size_guc_variables / 4; + int size_vars = size_guc_variables + size_guc_variables / 5; struct config_generic** guc_vars; if(size_vars == 0) + { size_vars = 100; - - guc_vars = (struct config_generic**) - malloc(size_vars * sizeof(struct config_generic*)); - - if (guc_variables != NULL) + guc_vars = (struct config_generic**) + guc_alloc(elevel, size_vars * sizeof(struct config_generic*)); + } + else { - memcpy(guc_vars, guc_variables, - num_guc_variables * sizeof(struct config_generic*)); - free(guc_variables); + guc_vars = (struct config_generic**) + guc_realloc(elevel, guc_variables, size_vars * sizeof(struct config_generic*)); } - guc_variables = guc_vars; + if(guc_vars == NULL) + /* + * Out of memory + */ + return false; + size_guc_variables = size_vars; + guc_variables = guc_vars; } guc_variables[num_guc_variables++] = var; qsort((void*) guc_variables, num_guc_variables, sizeof(struct config_generic*), guc_var_compare); + return true; } /* @@ -1941,15 +1969,22 @@ * to a valid custom variable class at this point. */ static struct config_string* -add_placeholder_variable(const char *name) +add_placeholder_variable(int elevel, const char *name) { size_t sz = sizeof(struct config_string) + sizeof(char*); - struct config_string* var = (struct config_string*)malloc(sz); - struct config_generic* gen = &var->gen; + struct config_generic* gen; + + struct config_string* var = (struct config_string*)guc_alloc(elevel, sz); + if(var == NULL) + return NULL; + gen = &var->gen; memset(var, 0, sz); - gen->name = strdup(name); + gen->name = guc_strdup(elevel, name); + if(gen->name == NULL) + return NULL; + gen->context = PGC_USERSET; gen->group = CUSTOM_OPTIONS; gen->short_desc = "GUC placeholder variable"; @@ -1960,7 +1995,8 @@ * no 'static' place to point to. */ var->variable = (char**)(var + 1); - add_guc_variable((struct config_generic*)var); + if(!add_guc_variable(elevel, (struct config_generic*)var)) + var = NULL; return var; } @@ -1969,7 +2005,7 @@ * else return NULL. */ static struct config_generic * -find_option(const char *name) +find_option(int elevel, const char *name) { const char *dot; const char **key = &name; @@ -1998,7 +2034,7 @@ for (i = 0; map_old_guc_names[i] != NULL; i += 2) { if (guc_name_compare(name, map_old_guc_names[i]) == 0) - return find_option(map_old_guc_names[i+1]); + return find_option(elevel, map_old_guc_names[i+1]); } /* Check if the name is qualified, and if so, check if the qualifier @@ -2009,7 +2045,7 @@ /* * Add a placeholder variable for this name */ - return (struct config_generic*)add_placeholder_variable(name); + return (struct config_generic*)add_placeholder_variable(elevel, name); /* Unknown name */ return NULL; @@ -2159,11 +2195,7 @@ break; } - str = strdup(conf->boot_val); - if (str == NULL) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + str = guc_strdup(FATAL, conf->boot_val); conf->reset_val = str; if (conf->assign_hook) @@ -2710,7 +2742,7 @@ else elevel = ERROR; - record = find_option(name); + record = find_option(elevel, name); if (record == NULL) { ereport(elevel, @@ -3142,14 +3174,9 @@ if (value) { - newval = strdup(value); + newval = guc_strdup(elevel, value); if (newval == NULL) - { - ereport(elevel, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); return false; - } if (record->context == PGC_USERLIMIT) { @@ -3200,14 +3227,9 @@ * make this case work the same as the normal * assignment case. */ - newval = strdup(conf->reset_val); + newval = guc_strdup(elevel, conf->reset_val); if (newval == NULL) - { - ereport(elevel, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); return false; - } source = conf->gen.reset_source; } else @@ -3338,7 +3360,7 @@ struct config_generic *record; static char buffer[256]; - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -3374,7 +3396,7 @@ struct config_generic *record; static char buffer[256]; - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -3430,7 +3452,7 @@ return NULL; /* Else get flags for the variable */ - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -3601,7 +3623,7 @@ if(res == NULL) { - add_guc_variable(variable); + add_guc_variable(ERROR, variable); return; } @@ -3658,7 +3680,7 @@ GucContext context, enum config_type type) { - gen->name = strdup(name); + gen->name = guc_strdup(ERROR, name); gen->context = context; gen->group = CUSTOM_OPTIONS; gen->short_desc = short_desc; @@ -3676,7 +3698,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_bool); - struct config_bool* var = (struct config_bool*)malloc(sz); + struct config_bool* var = (struct config_bool*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_BOOL); @@ -3698,7 +3720,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_int); - struct config_int* var = (struct config_int*)malloc(sz); + struct config_int* var = (struct config_int*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_INT); @@ -3720,7 +3742,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_real); - struct config_real* var = (struct config_real*)malloc(sz); + struct config_real* var = (struct config_real*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_REAL); @@ -3742,7 +3764,7 @@ GucShowHook show_hook) { size_t sz = sizeof(struct config_string); - struct config_string* var = (struct config_string*)malloc(sz); + struct config_string* var = (struct config_string*)guc_alloc(ERROR, sz); memset(var, 0, sz); init_custom_variable(&var->gen, name, short_desc, long_desc, context, PGC_STRING); @@ -3914,7 +3936,7 @@ { struct config_generic *record; - record = find_option(name); + record = find_option(ERROR, name); if (record == NULL) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), @@ -4281,16 +4303,15 @@ /* * Open file */ - new_filename = malloc(strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + + new_filename = guc_alloc(elevel, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + strlen(".new") + 2); - filename = malloc(strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); - if (new_filename == NULL || filename == NULL) - { - ereport(elevel, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + if(new_filename == NULL) return; - } + + filename = guc_alloc(elevel, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); + if (filename == NULL) + return; + sprintf(new_filename, "%s/" CONFIG_EXEC_PARAMS ".new", DataDir); sprintf(filename, "%s/" CONFIG_EXEC_PARAMS, DataDir); @@ -4402,9 +4423,9 @@ elog(FATAL, "invalid format of exec config params file"); } if (i == 0) - str = malloc(maxlen); + str = guc_alloc(FATAL, maxlen); else if (i == maxlen) - str = realloc(str, maxlen *= 2); + str = guc_realloc(FATAL, str, maxlen *= 2); str[i++] = ch; } while (ch != 0); @@ -4430,14 +4451,9 @@ /* * Open file */ - filename = malloc(strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); + filename = guc_alloc(ERROR, strlen(DataDir) + strlen(CONFIG_EXEC_PARAMS) + 2); if (filename == NULL) - { - ereport(ERROR, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); return; - } sprintf(filename, "%s/" CONFIG_EXEC_PARAMS, DataDir); fp = AllocateFile(filename, "r"); @@ -4459,7 +4475,7 @@ if ((varname = read_string_with_null(fp)) == NULL) break; - if ((record = find_option(varname)) == NULL) + if ((record = find_option(FATAL, varname)) == NULL) elog(FATAL, "failed to locate variable %s in exec config params file",varname); if ((varvalue = read_string_with_null(fp)) == NULL) elog(FATAL, "invalid format of exec config params file"); @@ -4500,28 +4516,16 @@ if (string[equal_pos] == '=') { - *name = malloc(equal_pos + 1); - if (!*name) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + *name = guc_alloc(FATAL, equal_pos + 1); strncpy(*name, string, equal_pos); (*name)[equal_pos] = '\0'; - *value = strdup(&string[equal_pos + 1]); - if (!*value) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + *value = guc_strdup(FATAL, &string[equal_pos + 1]); } else { /* no equal sign in string */ - *name = strdup(string); - if (!*name) - ereport(FATAL, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); + *name = guc_strdup(FATAL, string); *value = NULL; } ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [HACKERS] bug in GUC @ 2004-07-05 23:15 Tom Lane <[email protected]> parent: Thomas Hallgren <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Tom Lane @ 2004-07-05 23:15 UTC (permalink / raw) To: Thomas Hallgren <[email protected]>; +Cc: [email protected] Thomas Hallgren <[email protected]> writes: > Here's a patch dealing with the unchecked mallocs and strdups in guc.c. Applied with minor editorialization. Thanks. regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2004-07-05 23:15 UTC | newest] Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2004-06-24 05:27 bug in GUC Alvaro Herrera <[email protected]> 2004-06-24 06:49 ` Thomas Hallgren <[email protected]> 2004-06-24 09:06 ` Thomas Hallgren <[email protected]> 2004-06-24 14:02 ` Tom Lane <[email protected]> 2004-06-24 14:45 ` Thomas Hallgren <[email protected]> 2004-06-24 15:59 ` James Robinson <[email protected]> 2004-06-24 16:55 ` Alvaro Herrera <[email protected]> 2004-06-24 17:29 ` Tom Lane <[email protected]> 2004-06-27 18:45 ` Thomas Hallgren <[email protected]> 2004-07-05 23:15 ` Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox