Re: Missing NULL check after calling ecpg_strdup

[Álvaro Herrera]

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/