From mboxrd@z Thu Jan  1 00:00:00 1970
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Wed, 27 Nov 2019 14:52:53 +0900
Subject: [U-Boot] [PATCH v2 2/3] env: Tidy up some of the env code
In-Reply-To: <0102016e8e613d09-6624acd0-7882-4283-8512-d56fc73489ed-000000@eu-west-1.amazonses.com>
References: <20191121143240.122610-1-james.byrne@origamienergy.com>
 <0102016e8e613d09-6624acd0-7882-4283-8512-d56fc73489ed-000000@eu-west-1.amazonses.com>
Message-ID: <20191127055252.GN22427@linaro.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Thu, Nov 21, 2019 at 02:32:47PM +0000, James Byrne wrote:
> This commit tidies up a few things in the env code to make it safer and
> easier to extend:
> 
> - The hsearch_r() function took a 'struct env_entry' as its first
> parameter, but only used the 'key' and 'data' fields. Some callers would
> set the other fields, others would leave them undefined. Another
> disadvantage was that the struct 'data' member is a 'char *', but this
> function does not modify it, so it should be 'const char *'. To resolve
> these issues the function now takes explcit 'key' and 'data' parameters
> that are 'const char *', and all the callers have been modified.

I don't have a strong opinion here, but we'd rather maintain the current
interface. Yes, currently no users use any fields other than key/data,
but in the future, this function may be extended to accept additional
*search* parameters in a key, say flag?. At that time, we won't have to
change the interface again.

-Takahiro Akashi


> - Break up _do_env_set() so that it only does the argument handling,
> rename it to do_interactive_env_set() and use 'const char *' pointers
> for argv. The actual variable modification has been split out to two
> separate functions, do_env_remove() and do_env_update(), which can also
> be called from the programmatic version env_set(), meaning it no longer
> has to create fake command line parameters. The do_interactive_env_set()
> function is not required in SPL builds.
> 
> - Fix some warnings identified by checkpatch.pl
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
> 
> ---
> 
> Changes in v2:
> - Fix checkpatch.pl so that this patchset can pass without warnings.
> - Tidy up the underlying code before adding env_force_set()
> - Rename new function from env_force() to env_force_set()
> 
>  api/api.c             |   5 +-
>  cmd/nvedit.c          | 111 +++++++++++++++++++++++-------------------
>  drivers/tee/sandbox.c |  17 +++----
>  env/callback.c        |   7 +--
>  env/flags.c           |   7 +--
>  include/search.h      |   2 +-
>  lib/hashtable.c       |  83 ++++++++++++++++---------------
>  test/env/hashtable.c  |  23 ++-------
>  8 files changed, 119 insertions(+), 136 deletions(-)
> 
> diff --git a/api/api.c b/api/api.c
> index 71fa03804e..b950d8cbb7 100644
> --- a/api/api.c
> +++ b/api/api.c
> @@ -514,7 +514,7 @@ static int API_env_enum(va_list ap)
>  {
>  	int i, buflen;
>  	char *last, **next, *s;
> -	struct env_entry *match, search;
> +	struct env_entry *match;
>  	static char *var;
>  
>  	last = (char *)va_arg(ap, unsigned long);
> @@ -530,8 +530,7 @@ static int API_env_enum(va_list ap)
>  		s = strchr(var, '=');
>  		if (s != NULL)
>  			*s = 0;
> -		search.key = var;
> -		i = hsearch_r(search, ENV_FIND, &match, &env_htab, 0);
> +		i = hsearch_r(var, NULL, ENV_FIND, &match, &env_htab, 0);
>  		if (i == 0) {
>  			i = API_EINVAL;
>  			goto done;
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index 99a3bc57b1..b30669a45e 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -94,11 +94,9 @@ static int env_print(char *name, int flag)
>  	ssize_t len;
>  
>  	if (name) {		/* print a single name */
> -		struct env_entry e, *ep;
> +		struct env_entry *ep;
>  
> -		e.key = name;
> -		e.data = NULL;
> -		hsearch_r(e, ENV_FIND, &ep, &env_htab, flag);
> +		hsearch_r(name, NULL, ENV_FIND, &ep, &env_htab, flag);
>  		if (ep == NULL)
>  			return 0;
>  		len = printf("%s=%s\n", ep->key, ep->data);
> @@ -217,15 +215,55 @@ DONE:
>  #endif
>  #endif /* CONFIG_SPL_BUILD */
>  
> +static int do_env_remove(const char *name, int env_flag)
> +{
> +	int rc;
> +
> +	env_id++;
> +
> +	rc = hdelete_r(name, &env_htab, env_flag);
> +	return !rc;
> +}
> +
> +static int do_env_update(const char *name, const char *value, int env_flag)
> +{
> +	struct env_entry *ep;
> +
> +	env_id++;
> +
> +	hsearch_r(name, value, ENV_ENTER, &ep, &env_htab, env_flag);
> +	if (!ep) {
> +		printf("## Error inserting \"%s\" variable, errno=%d\n",
> +		       name, errno);
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +int env_set(const char *varname, const char *varvalue)
> +{
> +	/* before import into hashtable */
> +	if (!(gd->flags & GD_FLG_ENV_READY))
> +		return 1;
> +
> +	if (!varvalue || varvalue[0] == '\0')
> +		return do_env_remove(varname, H_PROGRAMMATIC);
> +
> +	return do_env_update(varname, varvalue, H_PROGRAMMATIC);
> +}
> +
> +#ifndef CONFIG_SPL_BUILD
>  /*
>   * Set a new environment variable,
>   * or replace or delete an existing one.
>   */
> -static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
> +static int do_interactive_env_set(int flag, int argc, const char * const argv[])
>  {
> -	int   i, len;
> -	char  *name, *value, *s;
> -	struct env_entry e, *ep;
> +	int env_flag = H_INTERACTIVE;
> +	int i, len, rc;
> +	const char *name;
> +	char *value, *s;
>  
>  	debug("Initial value for argc=%d\n", argc);
>  
> @@ -235,7 +273,7 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
>  #endif
>  
>  	while (argc > 1 && **(argv + 1) == '-') {
> -		char *arg = *++argv;
> +		const char *arg = *++argv;
>  
>  		--argc;
>  		while (*++arg) {
> @@ -257,12 +295,9 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
>  		return 1;
>  	}
>  
> -	env_id++;
> -
>  	/* Delete only ? */
>  	if (argc < 3 || argv[2] == NULL) {
> -		int rc = hdelete_r(name, &env_htab, env_flag);
> -		return !rc;
> +		return do_env_remove(name, env_flag);
>  	}
>  
>  	/*
> @@ -277,7 +312,7 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
>  		return 1;
>  	}
>  	for (i = 2, s = value; i < argc; ++i) {
> -		char *v = argv[i];
> +		const char *v = argv[i];
>  
>  		while ((*s++ = *v++) != '\0')
>  			;
> @@ -286,32 +321,12 @@ static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
>  	if (s != value)
>  		*--s = '\0';
>  
> -	e.key	= name;
> -	e.data	= value;
> -	hsearch_r(e, ENV_ENTER, &ep, &env_htab, env_flag);
> +	rc = do_env_update(name, value, env_flag);
>  	free(value);
> -	if (!ep) {
> -		printf("## Error inserting \"%s\" variable, errno=%d\n",
> -			name, errno);
> -		return 1;
> -	}
>  
> -	return 0;
> -}
> -
> -int env_set(const char *varname, const char *varvalue)
> -{
> -	const char * const argv[4] = { "setenv", varname, varvalue, NULL };
> -
> -	/* before import into hashtable */
> -	if (!(gd->flags & GD_FLG_ENV_READY))
> -		return 1;
> -
> -	if (varvalue == NULL || varvalue[0] == '\0')
> -		return _do_env_set(0, 2, (char * const *)argv, H_PROGRAMMATIC);
> -	else
> -		return _do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC);
> +	return rc;
>  }
> +#endif
>  
>  /**
>   * Set an environment variable to an integer value
> @@ -382,7 +397,7 @@ static int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	if (argc < 2)
>  		return CMD_RET_USAGE;
>  
> -	return _do_env_set(flag, argc, argv, H_INTERACTIVE);
> +	return do_interactive_env_set(flag, argc, (const char * const *)argv);
>  }
>  
>  /*
> @@ -393,7 +408,7 @@ int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
>  	char message[CONFIG_SYS_CBSIZE];
>  	int i, len, pos, size;
> -	char *local_args[4];
> +	const char *local_args[4];
>  	char *endptr;
>  
>  	local_args[0] = argv[0];
> @@ -460,7 +475,7 @@ int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	}
>  
>  	/* Continue calling setenv code */
> -	return _do_env_set(flag, len, local_args, H_INTERACTIVE);
> +	return do_interactive_env_set(flag, len, local_args);
>  }
>  #endif
>  
> @@ -643,12 +658,12 @@ static int do_env_edit(cmd_tbl_t *cmdtp, int flag, int argc,
>  	if (buffer[0] == '\0') {
>  		const char * const _argv[3] = { "setenv", argv[1], NULL };
>  
> -		return _do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE);
> +		return do_interactive_env_set(0, 2, _argv);
>  	} else {
>  		const char * const _argv[4] = { "setenv", argv[1], buffer,
>  			NULL };
>  
> -		return _do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE);
> +		return do_interactive_env_set(0, 3, _argv);
>  	}
>  }
>  #endif /* CONFIG_CMD_EDITENV */
> @@ -662,13 +677,11 @@ static int do_env_edit(cmd_tbl_t *cmdtp, int flag, int argc,
>  char *env_get(const char *name)
>  {
>  	if (gd->flags & GD_FLG_ENV_READY) { /* after import into hashtable */
> -		struct env_entry e, *ep;
> +		struct env_entry *ep;
>  
>  		WATCHDOG_RESET();
>  
> -		e.key	= name;
> -		e.data	= NULL;
> -		hsearch_r(e, ENV_FIND, &ep, &env_htab, 0);
> +		hsearch_r(name, NULL, ENV_FIND, &ep, &env_htab, 0);
>  
>  		return ep ? ep->data : NULL;
>  	}
> @@ -1262,14 +1275,12 @@ static int do_env_info(cmd_tbl_t *cmdtp, int flag,
>  static int do_env_exists(cmd_tbl_t *cmdtp, int flag, int argc,
>  		       char * const argv[])
>  {
> -	struct env_entry e, *ep;
> +	struct env_entry *ep;
>  
>  	if (argc < 2)
>  		return CMD_RET_USAGE;
>  
> -	e.key = argv[1];
> -	e.data = NULL;
> -	hsearch_r(e, ENV_FIND, &ep, &env_htab, 0);
> +	hsearch_r(argv[1], NULL, ENV_FIND, &ep, &env_htab, 0);
>  
>  	return (ep == NULL) ? 1 : 0;
>  }
> diff --git a/drivers/tee/sandbox.c b/drivers/tee/sandbox.c
> index 4b91e7db1b..92467ba89b 100644
> --- a/drivers/tee/sandbox.c
> +++ b/drivers/tee/sandbox.c
> @@ -79,7 +79,7 @@ static u32 ta_avb_invoke_func(struct udevice *dev, u32 func, uint num_params,
>  			      struct tee_param *params)
>  {
>  	struct sandbox_tee_state *state = dev_get_priv(dev);
> -	struct env_entry e, *ep;
> +	struct env_entry *ep;
>  	char *name;
>  	u32 res;
>  	uint slot;
> @@ -172,9 +172,7 @@ static u32 ta_avb_invoke_func(struct udevice *dev, u32 func, uint num_params,
>  		value = params[1].u.memref.shm->addr;
>  		value_sz = params[1].u.memref.size;
>  
> -		e.key = name;
> -		e.data = NULL;
> -		hsearch_r(e, ENV_FIND, &ep, &state->pstorage_htab, 0);
> +		hsearch_r(name, NULL, ENV_FIND, &ep, &state->pstorage_htab, 0);
>  		if (!ep)
>  			return TEE_ERROR_ITEM_NOT_FOUND;
>  
> @@ -196,15 +194,12 @@ static u32 ta_avb_invoke_func(struct udevice *dev, u32 func, uint num_params,
>  		value = params[1].u.memref.shm->addr;
>  		value_sz = params[1].u.memref.size;
>  
> -		e.key = name;
> -		e.data = NULL;
> -		hsearch_r(e, ENV_FIND, &ep, &state->pstorage_htab, 0);
> +		hsearch_r(name, NULL, ENV_FIND, &ep, &state->pstorage_htab, 0);
>  		if (ep)
> -			hdelete_r(e.key, &state->pstorage_htab, 0);
> +			hdelete_r(name, &state->pstorage_htab, 0);
>  
> -		e.key = name;
> -		e.data = value;
> -		hsearch_r(e, ENV_ENTER, &ep, &state->pstorage_htab, 0);
> +		hsearch_r(name, value, ENV_ENTER, &ep, &state->pstorage_htab,
> +			  0);
>  		if (!ep)
>  			return TEE_ERROR_OUT_OF_MEMORY;
>  
> diff --git a/env/callback.c b/env/callback.c
> index f0904cfdc5..e2296f9c5e 100644
> --- a/env/callback.c
> +++ b/env/callback.c
> @@ -92,13 +92,10 @@ static int clear_callback(struct env_entry *entry)
>   */
>  static int set_callback(const char *name, const char *value, void *priv)
>  {
> -	struct env_entry e, *ep;
> +	struct env_entry *ep;
>  	struct env_clbk_tbl *clbkp;
>  
> -	e.key	= name;
> -	e.data	= NULL;
> -	e.callback = NULL;
> -	hsearch_r(e, ENV_FIND, &ep, &env_htab, 0);
> +	hsearch_r(name, NULL, ENV_FIND, &ep, &env_htab, 0);
>  
>  	/* does the env variable actually exist? */
>  	if (ep != NULL) {
> diff --git a/env/flags.c b/env/flags.c
> index 418d6cc742..bc2348e1d2 100644
> --- a/env/flags.c
> +++ b/env/flags.c
> @@ -453,12 +453,9 @@ static int clear_flags(struct env_entry *entry)
>   */
>  static int set_flags(const char *name, const char *value, void *priv)
>  {
> -	struct env_entry e, *ep;
> +	struct env_entry *ep;
>  
> -	e.key	= name;
> -	e.data	= NULL;
> -	e.callback = NULL;
> -	hsearch_r(e, ENV_FIND, &ep, &env_htab, 0);
> +	hsearch_r(name, NULL, ENV_FIND, &ep, &env_htab, 0);
>  
>  	/* does the env variable actually exist? */
>  	if (ep != NULL) {
> diff --git a/include/search.h b/include/search.h
> index 0469a852e0..7243327f44 100644
> --- a/include/search.h
> +++ b/include/search.h
> @@ -68,7 +68,7 @@ void hdestroy_r(struct hsearch_data *htab);
>   * NULL.  If action is `ENV_ENTER' replace existing data (if any) with
>   * item.data.
>   * */
> -int hsearch_r(struct env_entry item, enum env_action action,
> +int hsearch_r(const char *key, const char *data, enum env_action action,
>  	      struct env_entry **retval, struct hsearch_data *htab, int flag);
>  
>  /*
> diff --git a/lib/hashtable.c b/lib/hashtable.c
> index 2caab0a4c6..d91f0d75e8 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -225,21 +225,24 @@ int hmatch_r(const char *match, int last_idx, struct env_entry **retval,
>   * Compare an existing entry with the desired key, and overwrite if the action
>   * is ENV_ENTER.  This is simply a helper function for hsearch_r().
>   */
> -static inline int _compare_and_overwrite_entry(struct env_entry item,
> -		enum env_action action, struct env_entry **retval,
> -		struct hsearch_data *htab, int flag, unsigned int hval,
> -		unsigned int idx)
> +static inline int _compare_and_overwrite_entry(const char *key,
> +					       const char *data,
> +					       enum env_action action,
> +					       struct env_entry **retval,
> +					       struct hsearch_data *htab,
> +					       int flag, unsigned int hval,
> +					       unsigned int idx)
>  {
> -	if (htab->table[idx].used == hval
> -	    && strcmp(item.key, htab->table[idx].entry.key) == 0) {
> +	if (htab->table[idx].used == hval &&
> +	    strcmp(key, htab->table[idx].entry.key) == 0) {
>  		/* Overwrite existing value? */
> -		if (action == ENV_ENTER && item.data) {
> +		if (action == ENV_ENTER && data) {
>  			/* check for permission */
>  			if (htab->change_ok != NULL && htab->change_ok(
> -			    &htab->table[idx].entry, item.data,
> +			    &htab->table[idx].entry, data,
>  			    env_op_overwrite, flag)) {
> -				debug("change_ok() rejected setting variable "
> -					"%s, skipping it!\n", item.key);
> +				debug("change_ok() rejected setting variable %s, skipping it!\n",
> +				      key);
>  				__set_errno(EPERM);
>  				*retval = NULL;
>  				return 0;
> @@ -247,17 +250,17 @@ static inline int _compare_and_overwrite_entry(struct env_entry item,
>  
>  			/* If there is a callback, call it */
>  			if (htab->table[idx].entry.callback &&
> -			    htab->table[idx].entry.callback(item.key,
> -			    item.data, env_op_overwrite, flag)) {
> -				debug("callback() rejected setting variable "
> -					"%s, skipping it!\n", item.key);
> +			    htab->table[idx].entry.callback(key,
> +			    data, env_op_overwrite, flag)) {
> +				debug("callback() rejected setting variable %s, skipping it!\n",
> +				      key);
>  				__set_errno(EINVAL);
>  				*retval = NULL;
>  				return 0;
>  			}
>  
>  			free(htab->table[idx].entry.data);
> -			htab->table[idx].entry.data = strdup(item.data);
> +			htab->table[idx].entry.data = strdup(data);
>  			if (!htab->table[idx].entry.data) {
>  				__set_errno(ENOMEM);
>  				*retval = NULL;
> @@ -272,12 +275,12 @@ static inline int _compare_and_overwrite_entry(struct env_entry item,
>  	return -1;
>  }
>  
> -int hsearch_r(struct env_entry item, enum env_action action,
> +int hsearch_r(const char *key, const char *data, enum env_action action,
>  	      struct env_entry **retval, struct hsearch_data *htab, int flag)
>  {
>  	unsigned int hval;
>  	unsigned int count;
> -	unsigned int len = strlen(item.key);
> +	unsigned int len = strlen(key);
>  	unsigned int idx;
>  	unsigned int first_deleted = 0;
>  	int ret;
> @@ -287,7 +290,7 @@ int hsearch_r(struct env_entry item, enum env_action action,
>  	count = len;
>  	while (count-- > 0) {
>  		hval <<= 4;
> -		hval += item.key[count];
> +		hval += key[count];
>  	}
>  
>  	/*
> @@ -312,8 +315,8 @@ int hsearch_r(struct env_entry item, enum env_action action,
>  		    && !first_deleted)
>  			first_deleted = idx;
>  
> -		ret = _compare_and_overwrite_entry(item, action, retval, htab,
> -			flag, hval, idx);
> +		ret = _compare_and_overwrite_entry(key, data, action, retval,
> +						   htab, flag, hval, idx);
>  		if (ret != -1)
>  			return ret;
>  
> @@ -345,8 +348,9 @@ int hsearch_r(struct env_entry item, enum env_action action,
>  				first_deleted = idx;
>  
>  			/* If entry is found use it. */
> -			ret = _compare_and_overwrite_entry(item, action, retval,
> -				htab, flag, hval, idx);
> +			ret = _compare_and_overwrite_entry(key, data, action,
> +							   retval, htab, flag,
> +							   hval, idx);
>  			if (ret != -1)
>  				return ret;
>  		}
> @@ -367,14 +371,14 @@ int hsearch_r(struct env_entry item, enum env_action action,
>  
>  		/*
>  		 * Create new entry;
> -		 * create copies of item.key and item.data
> +		 * create copies of key and data
>  		 */
>  		if (first_deleted)
>  			idx = first_deleted;
>  
>  		htab->table[idx].used = hval;
> -		htab->table[idx].entry.key = strdup(item.key);
> -		htab->table[idx].entry.data = strdup(item.data);
> +		htab->table[idx].entry.key = strdup(key);
> +		htab->table[idx].entry.data = strdup(data);
>  		if (!htab->table[idx].entry.key ||
>  		    !htab->table[idx].entry.data) {
>  			__set_errno(ENOMEM);
> @@ -391,10 +395,10 @@ int hsearch_r(struct env_entry item, enum env_action action,
>  
>  		/* check for permission */
>  		if (htab->change_ok != NULL && htab->change_ok(
> -		    &htab->table[idx].entry, item.data, env_op_create, flag)) {
> -			debug("change_ok() rejected setting variable "
> -				"%s, skipping it!\n", item.key);
> -			_hdelete(item.key, htab, &htab->table[idx].entry, idx);
> +		    &htab->table[idx].entry, data, env_op_create, flag)) {
> +			debug("change_ok() rejected setting variable %s, skipping it!\n",
> +			      key);
> +			_hdelete(key, htab, &htab->table[idx].entry, idx);
>  			__set_errno(EPERM);
>  			*retval = NULL;
>  			return 0;
> @@ -402,11 +406,11 @@ int hsearch_r(struct env_entry item, enum env_action action,
>  
>  		/* If there is a callback, call it */
>  		if (htab->table[idx].entry.callback &&
> -		    htab->table[idx].entry.callback(item.key, item.data,
> +		    htab->table[idx].entry.callback(key, data,
>  		    env_op_create, flag)) {
> -			debug("callback() rejected setting variable "
> -				"%s, skipping it!\n", item.key);
> -			_hdelete(item.key, htab, &htab->table[idx].entry, idx);
> +			debug("callback() rejected setting variable %s, skipping it!\n",
> +			      key);
> +			_hdelete(key, htab, &htab->table[idx].entry, idx);
>  			__set_errno(EINVAL);
>  			*retval = NULL;
>  			return 0;
> @@ -449,14 +453,12 @@ static void _hdelete(const char *key, struct hsearch_data *htab,
>  
>  int hdelete_r(const char *key, struct hsearch_data *htab, int flag)
>  {
> -	struct env_entry e, *ep;
> +	struct env_entry *ep;
>  	int idx;
>  
>  	debug("hdelete: DELETE key \"%s\"\n", key);
>  
> -	e.key = (char *)key;
> -
> -	idx = hsearch_r(e, ENV_FIND, &ep, htab, 0);
> +	idx = hsearch_r(key, NULL, ENV_FIND, &ep, htab, 0);
>  	if (idx == 0) {
>  		__set_errno(ESRCH);
>  		return 0;	/* not found */
> @@ -871,7 +873,7 @@ int himport_r(struct hsearch_data *htab,
>  	}
>  	/* Parse environment; allow for '\0' and 'sep' as separators */
>  	do {
> -		struct env_entry e, *rv;
> +		struct env_entry *rv;
>  
>  		/* skip leading white space */
>  		while (isblank(*dp))
> @@ -928,10 +930,7 @@ int himport_r(struct hsearch_data *htab,
>  			continue;
>  
>  		/* enter into hash table */
> -		e.key = name;
> -		e.data = value;
> -
> -		hsearch_r(e, ENV_ENTER, &rv, htab, flag);
> +		hsearch_r(name, value, ENV_ENTER, &rv, htab, flag);
>  		if (rv == NULL)
>  			printf("himport_r: can't insert \"%s=%s\" into hash table\n",
>  				name, value);
> diff --git a/test/env/hashtable.c b/test/env/hashtable.c
> index 5242c4cc3e..36a59d859a 100644
> --- a/test/env/hashtable.c
> +++ b/test/env/hashtable.c
> @@ -18,17 +18,12 @@ static int htab_fill(struct unit_test_state *uts,
>  		     struct hsearch_data *htab, size_t size)
>  {
>  	size_t i;
> -	struct env_entry item;
>  	struct env_entry *ritem;
>  	char key[20];
>  
>  	for (i = 0; i < size; i++) {
>  		sprintf(key, "%d", (int)i);
> -		item.callback = NULL;
> -		item.data = key;
> -		item.flags = 0;
> -		item.key = key;
> -		ut_asserteq(1, hsearch_r(item, ENV_ENTER, &ritem, htab, 0));
> +		ut_asserteq(1, hsearch_r(key, key, ENV_ENTER, &ritem, htab, 0));
>  	}
>  
>  	return 0;
> @@ -38,17 +33,12 @@ static int htab_check_fill(struct unit_test_state *uts,
>  			   struct hsearch_data *htab, size_t size)
>  {
>  	size_t i;
> -	struct env_entry item;
>  	struct env_entry *ritem;
>  	char key[20];
>  
>  	for (i = 0; i < size; i++) {
>  		sprintf(key, "%d", (int)i);
> -		item.callback = NULL;
> -		item.flags = 0;
> -		item.data = key;
> -		item.key = key;
> -		hsearch_r(item, ENV_FIND, &ritem, htab, 0);
> +		hsearch_r(key, key, ENV_FIND, &ritem, htab, 0);
>  		ut_assert(ritem);
>  		ut_asserteq_str(key, ritem->key);
>  		ut_asserteq_str(key, ritem->data);
> @@ -61,20 +51,15 @@ static int htab_create_delete(struct unit_test_state *uts,
>  			      struct hsearch_data *htab, size_t iterations)
>  {
>  	size_t i;
> -	struct env_entry item;
>  	struct env_entry *ritem;
>  	char key[20];
>  
>  	for (i = 0; i < iterations; i++) {
>  		sprintf(key, "cd-%d", (int)i);
> -		item.callback = NULL;
> -		item.flags = 0;
> -		item.data = key;
> -		item.key = key;
> -		hsearch_r(item, ENV_ENTER, &ritem, htab, 0);
> +		hsearch_r(key, key, ENV_ENTER, &ritem, htab, 0);
>  		ritem = NULL;
>  
> -		hsearch_r(item, ENV_FIND, &ritem, htab, 0);
> +		hsearch_r(key, key, ENV_FIND, &ritem, htab, 0);
>  		ut_assert(ritem);
>  		ut_asserteq_str(key, ritem->key);
>  		ut_asserteq_str(key, ritem->data);
> -- 
> 2.24.0
>