[U-Boot] [PATCH v3 2/6] env: make himport_r() selective on variables

[Gerlando Falauto]

On 04/02/2012 08:57 PM, Marek Vasut wrote:
> Dear Gerlando Falauto,
>
>> Add 2 new arguments to himport_r():
>>
>>   o "nvars", "vars": number and list of variables to take into account
>>     (0 means ALL)
>>
>
> Looks nice and clean. But man, these patches are tough to review (not because
> you made bad job, just because they're hard code), lot of headache from them ;-)

Tell me about it. I've been reworking them several times, usually after 
several weeks from posting them. Long enough to have forgotten 
everything, and to give you a bad headache.
So I really appreciate your prompt feedback, thanks!

Gerlando

P.S. For the whole process I am using interactive rebase, which is 
painful enough. If anyone knows of any other way to "patch the patches", 
some advice would be very much appreciated... :-)

>> NOTE: This patch does not change the current behaviour.
>>
>> Signed-off-by: Gerlando Falauto<gerlando.falauto at keymile.com>
>> ---
>>   common/cmd_nvedit.c |    3 ++-
>>   common/env_common.c |    6 ++++--
>>   include/search.h    |    6 +++++-
>>   lib/hashtable.c     |   27 ++++++++++++++++++++++++++-
>>   4 files changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
>> index e762e76..59668a6 100644
>> --- a/common/cmd_nvedit.c
>> +++ b/common/cmd_nvedit.c
>> @@ -930,7 +930,8 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
>>   		addr = (char *)ep->data;
>>   	}
>>
>> -	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR) == 0) {
>> +	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
>> +			0, NULL) == 0) {
>>   		error("Environment import failed: errno = %d\n", errno);
>>   		return 1;
>>   	}
>> diff --git a/common/env_common.c b/common/env_common.c
>> index c33d22d..a93c062 100644
>> --- a/common/env_common.c
>> +++ b/common/env_common.c
>> @@ -182,7 +182,8 @@ void set_default_env(const char *s)
>>   	}
>>
>>   	if (himport_r(&env_htab, (char *)default_environment,
>> -			sizeof(default_environment), '\0', 0) == 0)
>> +			sizeof(default_environment), '\0', 0,
>> +			0, NULL) == 0)
>>   		error("Environment import failed: errno = %d\n", errno);
>>
>>   	gd->flags |= GD_FLG_ENV_READY;
>> @@ -207,7 +208,8 @@ int env_import(const char *buf, int check)
>>   		}
>>   	}
>>
>> -	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0)) {
>> +	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
>> +			0, NULL)) {
>>   		gd->flags |= GD_FLG_ENV_READY;
>>   		return 1;
>>   	}
>> diff --git a/include/search.h b/include/search.h
>> index a4a5ef4..94d75fc 100644
>> --- a/include/search.h
>> +++ b/include/search.h
>> @@ -94,9 +94,13 @@ extern ssize_t hexport_r(struct hsearch_data *__htab,
>>   		     const char __sep, char **__resp, size_t __size,
>>   		     int argc, char * const argv[]);
>>
>> +/*
>> + * nvars: length of vars array
>> + * vars: array of strings (variable names) to import (nvars == 0 means
>> all) + */
>>   extern int himport_r(struct hsearch_data *__htab,
>>   		     const char *__env, size_t __size, const char __sep,
>> -		     int __flag);
>> +		     int __flag, int nvars, char * const vars[]);
>>
>>   /* Flags for himport_r() */
>>   #define	H_NOCLEAR	(1<<  0) /* do not clear hash table before
> importing */
>> diff --git a/lib/hashtable.c b/lib/hashtable.c
>> index abd61c8..0610e86 100644
>> --- a/lib/hashtable.c
>> +++ b/lib/hashtable.c
>> @@ -603,6 +603,24 @@ ssize_t hexport_r(struct hsearch_data *htab, const
>> char sep, * himport()
>>    */
>>
>> +/* Check whether variable name is amongst vars[] */
>> +static int is_var_in_set(const char *name, int nvars, char * const vars[])
>> +{
>> +	int i = 0;
>> +
>> +	/* No variables specified means process all of them */
>> +	if (nvars == 0)
>> +		return 1;
>> +
>> +	for (i = 0; i<  nvars; i++) {
>> +		if (!strcmp(name, vars[i]))
>> +			return 1;
>> +	}
>> +	debug("Skipping non-listed variable %s\n", name);
>> +
>> +	return 0;
>> +}
>> +
>>   /*
>>    * Import linearized data into hash table.
>>    *
>> @@ -639,7 +657,8 @@ ssize_t hexport_r(struct hsearch_data *htab, const char
>> sep, */
>>
>>   int himport_r(struct hsearch_data *htab,
>> -	      const char *env, size_t size, const char sep, int flag)
>> +		const char *env, size_t size, const char sep, int flag,
>> +		int nvars, char * const vars[])
>>   {
>>   	char *data, *sp, *dp, *name, *value;
>>
>> @@ -726,6 +745,8 @@ int himport_r(struct hsearch_data *htab,
>>   			*dp++ = '\0';	/* terminate name */
>>
>>   			debug("DELETE CANDIDATE: \"%s\"\n", name);
>> +			if (!is_var_in_set(name, nvars, vars))
>> +				continue;
>>
>>   			if (hdelete_r(name, htab) == 0)
>>   				debug("DELETE ERROR
> ##############################\n");
>> @@ -743,6 +764,10 @@ int himport_r(struct hsearch_data *htab,
>>   		*sp++ = '\0';	/* terminate value */
>>   		++dp;
>>
>> +		/* Skip variables which are not supposed to be processed */
>> +		if (!is_var_in_set(name, nvars, vars))
>> +			continue;
>> +
>>   		/* enter into hash table */
>>   		e.key = name;
>>   		e.data = value;