Re: Missing NULL check after calling ecpg_strdup - Mailing list pgsql-hackers

On 2025-Jul-14, Aleksander Alekseev wrote:

> @@ -460,7 +461,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)
> +
> +    /* Allocate memory for connection name */
> +    if (connection_name != NULL)
> +        this->name = ecpg_strdup(connection_name, lineno);
> +    else
> +        this->name = ecpg_strdup(realname, lineno);
> +
> +    /*
> +     * Note that NULL is a correct value for realname and ecpg_strdup(NULL,
> +     * ...) just returns NULL. For named reasons the ckecks for this->name are
> +     * a bit complicated here.
> +     */
> +    if (conn_keywords == NULL || conn_values == NULL ||
> +        (connection_name != NULL && this->name == NULL) ||    /* first call failed */
> +        (connection_name == NULL && realname != NULL && this->name == NULL))    /* second call failed */
>      {
>          if (host)
>              ecpg_free(host);

This looks super baroque.  Why not simplify a bit instead?  Maybe
something like

@@ -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);


-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/