[DTrace-devel] [PATCH v3 04/11] htab: have dt_htab_destroy del all the elements

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

On 11/1/21 10:29 AM, Nick Alcock wrote:
> This means that a dt_htab_destroy of a non-empty htab doesn't leak memory
> and leave all its former elements full of wild pointers.
>
> Also add some comments to dt_htab_delete given how many mistakes I made
> reading it.
>
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>   libdtrace/dt_htab.c | 29 ++++++++++++++++++++++++++++-
>   1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_htab.c b/libdtrace/dt_htab.c
> index e4876fc93042..b04a70e88fc2 100644
> --- a/libdtrace/dt_htab.c
> +++ b/libdtrace/dt_htab.c
> @@ -73,13 +73,29 @@ dt_htab_t *dt_htab_create(dtrace_hdl_t *dtp, dt_htab_ops_t *ops)
>   }
>   
>   /*
> - * Destroy a hashtable.
> + * Destroy a hashtable, deleting all its entries first.
>    */
>   void dt_htab_destroy(dtrace_hdl_t *dtp, dt_htab_t *htab)
>   {
> +	size_t		i;
> +
>   	if (!htab)
>   		return;
>   
> +	for (i = 0; i < htab->size; i++) {
> +		dt_hbucket_t	*bucket = htab->tab[i];
> +
> +		while (bucket) {
> +			dt_hbucket_t *obucket = bucket;
> +
> +			while (bucket->head)
> +				bucket->head = htab->ops->del(bucket->head,
> +				    bucket->head);
> +			bucket = bucket->next;
> +			free(obucket);
> +		};
> +	}
> +
>   	dt_free(dtp, htab->tab);
>   	dt_free(dtp, htab);
>   }
> @@ -193,6 +209,10 @@ int dt_htab_delete(dt_htab_t *htab, void *entry)
>   	dt_hbucket_t	*bucket;
>   	void		*head;
>   
> +	/*
> +	 * Find the right bucket in cases of hash collision.
> +	 */
> +
>   	for (bucket = htab->tab[idx]; bucket; bucket = bucket->next) {
>   		if (htab->ops->cmp(bucket->head, entry) == 0)
>   			break;
> @@ -201,9 +221,16 @@ int dt_htab_delete(dt_htab_t *htab, void *entry)
>   	if (bucket == NULL)
>   		return -ENOENT;
>   
> +	/*
> +	 * Delete the specified entry, now known to be in this bucket.
> +	 */
>   	head = htab->ops->del(bucket->head, entry);
>   	bucket->nentries--;
>   	htab->nentries--;
> +
> +	/*
> +	 * Deal with a now-empty bucket.
> +	 */
>   	if (!head) {
>   		dt_hbucket_t	*b = htab->tab[idx];
>