Hi Alvaro, > This looks super baroque. Why not simplify a bit instead? Maybe > something like > > [...]
Fair point. Here is the corrected patch.
From 35cbf8fa22d0a1e9d1b46784a83adfdfd5c675fb Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <aleksan...@timescale.com> Date: Fri, 11 Jul 2025 17:59:50 +0300 Subject: [PATCH v4] Add proper checks for ecpg_strdup() return value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Author: Evgeniy Gorbanev <gorbanyo...@basealt.ru> Author: Aleksander Alekseev <aleksan...@tigerdata.com> Reviewed-by: Álvaro Herrera <alvhe...@kurilemu.de> Discussion: https://postgr.es/m/a6b193c1-6994-4d9c-9059-aca4aaf41...@basealt.ru --- src/interfaces/ecpg/ecpglib/connect.c | 29 +++++++++++++++++++------ src/interfaces/ecpg/ecpglib/execute.c | 17 +++++++++++++++ src/interfaces/ecpg/ecpglib/prepare.c | 31 +++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c index 2bbb70333dc..af25582abbd 100644 --- a/src/interfaces/ecpg/ecpglib/connect.c +++ b/src/interfaces/ecpg/ecpglib/connect.c @@ -58,7 +58,8 @@ ecpg_get_connection_nr(const char *connection_name) for (con = all_connections; con != NULL; con = con->next) { - if (strcmp(connection_name, con->name) == 0) + /* Check for NULL to prevent segfault */ + if (con->name != NULL && strcmp(connection_name, con->name) == 0) break; } ret = con; @@ -267,6 +268,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p *options = NULL; const char **conn_keywords; const char **conn_values; + bool strdup_failed; if (sqlca == NULL) { @@ -460,7 +462,21 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p */ conn_keywords = (const char **) ecpg_alloc((connect_params + 1) * sizeof(char *), lineno); conn_values = (const char **) ecpg_alloc(connect_params * sizeof(char *), lineno); - if (conn_keywords == NULL || conn_values == NULL) + + /* Decide on a connection name */ + strdup_failed = false; + if (connection_name != NULL || realname != NULL) + { + this->name = ecpg_strdup(connection_name ? connection_name : realname, + lineno); + if (this->name == NULL) + strdup_failed = true; + } + else + this->name = NULL; + + /* Deal with any failed allocations above */ + if (conn_keywords == NULL || conn_values == NULL || strdup_failed) { if (host) ecpg_free(host); @@ -476,6 +492,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p ecpg_free(conn_keywords); if (conn_values) ecpg_free(conn_values); + if (this->name) + ecpg_free(this->name); free(this); return false; } @@ -510,17 +528,14 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p ecpg_free(conn_keywords); if (conn_values) ecpg_free(conn_values); + if (this->name) + ecpg_free(this->name); free(this); return false; } } #endif - if (connection_name != NULL) - this->name = ecpg_strdup(connection_name, lineno); - else - this->name = ecpg_strdup(realname, lineno); - this->cache_head = NULL; this->prep_stmts = NULL; diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index f52da06de9a..6524c646c13 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -2030,7 +2030,14 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, statement_type = ECPGst_execute; } else + { stmt->command = ecpg_strdup(query, lineno); + if (!stmt->command) + { + ecpg_do_epilogue(stmt); + return false; + } + } stmt->name = NULL; @@ -2043,6 +2050,11 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, { stmt->name = stmt->command; stmt->command = ecpg_strdup(command, lineno); + if (!stmt->command) + { + ecpg_do_epilogue(stmt); + return false; + } } else { @@ -2176,6 +2188,11 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, if (!is_prepared_name_set && stmt->statement_type == ECPGst_prepare) { stmt->name = ecpg_strdup(var->value, lineno); + if (!stmt->name) + { + ecpg_do_epilogue(stmt); + return false; + } is_prepared_name_set = true; } } diff --git a/src/interfaces/ecpg/ecpglib/prepare.c b/src/interfaces/ecpg/ecpglib/prepare.c index ea1146f520f..7b6019deed7 100644 --- a/src/interfaces/ecpg/ecpglib/prepare.c +++ b/src/interfaces/ecpg/ecpglib/prepare.c @@ -86,8 +86,21 @@ ecpg_register_prepared_stmt(struct statement *stmt) prep_stmt->lineno = lineno; prep_stmt->connection = con; prep_stmt->command = ecpg_strdup(stmt->command, lineno); + if (!prep_stmt->command) + { + ecpg_free(prep_stmt); + ecpg_free(this); + return false; + } prep_stmt->inlist = prep_stmt->outlist = NULL; this->name = ecpg_strdup(stmt->name, lineno); + if (!this->name) + { + ecpg_free(prep_stmt->command); + ecpg_free(prep_stmt); + ecpg_free(this); + return false; + } this->stmt = prep_stmt; this->prepared = true; @@ -178,6 +191,12 @@ prepare_common(int lineno, struct connection *con, const char *name, const char stmt->lineno = lineno; stmt->connection = con; stmt->command = ecpg_strdup(variable, lineno); + if (!stmt->command) + { + ecpg_free(stmt); + ecpg_free(this); + return false; + } stmt->inlist = stmt->outlist = NULL; /* if we have C variables in our statement replace them with '?' */ @@ -185,6 +204,13 @@ prepare_common(int lineno, struct connection *con, const char *name, const char /* add prepared statement to our list */ this->name = ecpg_strdup(name, lineno); + if (!this->name) + { + ecpg_free(stmt->command); + ecpg_free(stmt); + ecpg_free(this); + return false; + } this->stmt = stmt; /* and finally really prepare the statement */ @@ -541,6 +567,8 @@ AddStmtToCache(int lineno, /* line # of statement */ entry = &stmtCacheEntries[entNo]; entry->lineno = lineno; entry->ecpgQuery = ecpg_strdup(ecpgQuery, lineno); + if (!entry->ecpgQuery) + return -1; entry->connection = connection; entry->execs = 0; memcpy(entry->stmtID, stmtID, sizeof(entry->stmtID)); @@ -595,6 +623,9 @@ ecpg_auto_prepare(int lineno, const char *connection_name, const int compat, cha *name = ecpg_strdup(stmtID, lineno); } + if (!*name) + return false; + /* increase usage counter */ stmtCacheEntries[entNo].execs++; -- 2.43.0
