From: Otto Moerbeek <otto@drijf.net>
Subject: Re: dhcpd(8): don't avoid free()
To: David Gwynne <david@gwynne.id.au>
Cc: tech@openbsd.org
Date: Fri, 30 May 2025 07:45:34 +0200

On Fri, May 30, 2025 at 03:28:54PM +1000, David Gwynne wrote:

> dhcpd seems to keep it's own free lists for a couple of data
> structures.  my best guess for why it would do this is to avoid
> malloc overhead.  however, responding to dhcp packets is not what
> i would consider a webscale problem^W^W^Wperformance critical, so
> this optimisation is unecessary.
> 
> ok?

You can also get rid of the next field in struct lease_state now.

With that, OK

	-Otto
	
> 
> Index: alloc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dhcpd/alloc.c,v
> diff -u -p -r1.15 alloc.c
> --- alloc.c	13 Feb 2017 19:13:14 -0000	1.15
> +++ alloc.c	30 May 2025 05:26:08 -0000
> @@ -55,30 +55,22 @@
>  #include "dhcpd.h"
>  #include "log.h"
>  
> -struct lease_state *free_lease_states;
> -struct tree_cache *free_tree_caches;
> -
>  struct tree_cache *
>  new_tree_cache(char *name)
>  {
>  	struct tree_cache *rval;
>  
> -	if (free_tree_caches) {
> -		rval = free_tree_caches;
> -		free_tree_caches = (struct tree_cache *)(rval->value);
> -	} else {
> -		rval = calloc(1, sizeof(struct tree_cache));
> -		if (!rval)
> -			fatalx("unable to allocate tree cache for %s.", name);
> -	}
> +	rval = calloc(1, sizeof(*rval));
> +	if (rval == NULL)
> +		fatalx("unable to allocate tree cache for %s.", name);
> +
>  	return (rval);
>  }
>  
>  void
>  free_tree_cache(struct tree_cache *ptr)
>  {
> -	ptr->value = (unsigned char *)free_tree_caches;
> -	free_tree_caches = ptr;
> +	free(ptr);
>  }
>  
>  struct lease_state *
> @@ -86,14 +78,9 @@ new_lease_state(char *name)
>  {
>  	struct lease_state *rval;
>  
> -	if (free_lease_states) {
> -		rval = free_lease_states;
> -		free_lease_states = free_lease_states->next;
> -	} else {
> -		rval = calloc(1, sizeof(struct lease_state));
> -		if (!rval)
> -			fatalx("unable to allocate lease state for %s.", name);
> -	}
> +	rval = calloc(1, sizeof(*rval));
> +	if (rval == NULL)
> +		fatalx("unable to allocate lease state for %s.", name);
>  
>  	return (rval);
>  }
> @@ -102,6 +89,5 @@ void
>  free_lease_state(struct lease_state *ptr, char *name)
>  {
>  	free(ptr->prl);
> -	ptr->next = free_lease_states;
> -	free_lease_states = ptr;
> +	free(ptr);
>  }
>