[Mesa-dev] [PATCH 6/6] mesa: Fix hash table leak

[Ian Romanick]

On 05/24/2012 06:29 PM, Stéphane Marchesin wrote:
> From: Antoine Labour<piman at chromium.org>
>
> When a value was replaced, the new key was strdup'd and leaked.
> To fix this, we modify the hash table implementation to return
> whether the value was replaced and free() the (now useless)
> duplicate string.

This looks good, but it should return bool instead of int.

> ---
>   src/mesa/program/hash_table.c |    5 +++--
>   src/mesa/program/hash_table.h |   16 ++++++++++++----
>   2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/program/hash_table.c b/src/mesa/program/hash_table.c
> index dc8563a..067c20b 100644
> --- a/src/mesa/program/hash_table.c
> +++ b/src/mesa/program/hash_table.c
> @@ -149,7 +149,7 @@ hash_table_insert(struct hash_table *ht, void *data, const void *key)
>       insert_at_head(&  ht->buckets[bucket],&  node->link);
>   }
>
> -void
> +int
>   hash_table_replace(struct hash_table *ht, void *data, const void *key)
>   {
>       const unsigned hash_value = (*ht->hash)(key);
> @@ -162,7 +162,7 @@ hash_table_replace(struct hash_table *ht, void *data, const void *key)
>
>          if ((*ht->compare)(hn->key, key) == 0) {
>   	  hn->data = data;
> -	  return;
> +	  return 1;
>          }
>       }
>
> @@ -172,6 +172,7 @@ hash_table_replace(struct hash_table *ht, void *data, const void *key)
>       hn->key = key;
>
>       insert_at_head(&  ht->buckets[bucket],&  hn->link);
> +    return 0;
>   }
>
>   void
> diff --git a/src/mesa/program/hash_table.h b/src/mesa/program/hash_table.h
> index bcf65df..a14bf13 100644
> --- a/src/mesa/program/hash_table.h
> +++ b/src/mesa/program/hash_table.h
> @@ -114,6 +114,10 @@ extern void hash_table_insert(struct hash_table *ht, void *data,
>   /**
>    * Add an element to a hash table with replacement
>    *
> + * \return
> + * 1 if it did replace the the value (in which case the old key is kept), 0 if
> + * it did not replace the value (in which case the new key is kept).
> + *
>    * \warning
>    * If \c key is already in the hash table, \c data will \b replace the most
>    * recently inserted \c data (see the warning in \c hash_table_insert) for
> @@ -121,7 +125,7 @@ extern void hash_table_insert(struct hash_table *ht, void *data,
>    *
>    * \sa hash_table_insert
>    */
> -extern void hash_table_replace(struct hash_table *ht, void *data,
> +extern int hash_table_replace(struct hash_table *ht, void *data,
>       const void *key);
>
>   /**
> @@ -219,6 +223,7 @@ public:
>       */
>      void clear()
>      {
> +      hash_table_call_foreach(this->ht, delete_key, NULL);
>         hash_table_clear(this->ht);
>      }
>
> @@ -258,9 +263,12 @@ public:
>          * because UINT_MAX+1 = 0.
>          */
>         assert(value != UINT_MAX);
> -      hash_table_replace(this->ht,
> -			 (void *) (intptr_t) (value + 1),
> -			 strdup(key));
> +      char *dup_key = strdup(key);
> +      int result = hash_table_replace(this->ht,
> +				      (void *) (intptr_t) (value + 1),
> +				      dup_key);
> +      if (result)
> +	 free(dup_key);
>      }
>
>   private: