diff mbox

[U-Boot,v3,3/6] env: add check/apply logic to himport_r()

Message ID 1333391204-16318-4-git-send-email-gerlando.falauto@keymile.com
State Accepted
Commit c5983592e912835fe9ed00b9d98b05580c460eae
Headers show

Commit Message

Gerlando Falauto April 2, 2012, 6:26 p.m. UTC
Change hashtable so that a callback function will decide whether a
variable can be overwritten, and possibly apply the changes.

So add a new field to struct hsearch_data:

 o "apply" callback function to check whether a variable can be
    overwritten, and possibly immediately apply the changes;
    when NULL, no check is performed.

And a new argument to himport_r():
 o "do_apply": whether to call the apply callback function

NOTE: This patch does not change the current behavior.

Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
---
 common/cmd_nvedit.c   |    3 ++-
 common/env_common.c   |    8 +++++---
 include/environment.h |    9 +++++++++
 include/search.h      |   14 +++++++++++++-
 lib/hashtable.c       |   20 +++++++++++++++++++-
 5 files changed, 48 insertions(+), 6 deletions(-)

Comments

Marek Vasut April 2, 2012, 7 p.m. UTC | #1
Dear Gerlando Falauto,

> Change hashtable so that a callback function will decide whether a
> variable can be overwritten, and possibly apply the changes.
> 
> So add a new field to struct hsearch_data:
> 
>  o "apply" callback function to check whether a variable can be
>     overwritten, and possibly immediately apply the changes;
>     when NULL, no check is performed.
> 
> And a new argument to himport_r():
>  o "do_apply": whether to call the apply callback function
> 
> NOTE: This patch does not change the current behavior.
> 
> Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> ---
>  common/cmd_nvedit.c   |    3 ++-
>  common/env_common.c   |    8 +++++---
>  include/environment.h |    9 +++++++++
>  include/search.h      |   14 +++++++++++++-
>  lib/hashtable.c       |   20 +++++++++++++++++++-
>  5 files changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> index 59668a6..fc6165f 100644
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -201,6 +201,7 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
>   * environment variable, then (if successful) apply the changes to
> internals so * to make them effective.  Code for this function was taken
> out of * _do_env_set(), which now calls it instead.
> + * Also called as a callback function by himport_r().
>   * Returns 0 in case of success, 1 in case of failure.
>   * When (flag & H_FORCE) is set, do not print out any error message and
> force * overwriting of write-once variables.
> @@ -931,7 +932,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
>  	}
> 
>  	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
> -			0, NULL) == 0) {
> +			0, NULL, 0 /* do_apply */) == 0) {
>  		error("Environment import failed: errno = %d\n", errno);
>  		return 1;
>  	}
> diff --git a/common/env_common.c b/common/env_common.c
> index a93c062..5558c1c 100644
> --- a/common/env_common.c
> +++ b/common/env_common.c
> @@ -122,7 +122,9 @@ const uchar default_environment[] = {
>  	"\0"
>  };
> 
> -struct hsearch_data env_htab;
> +struct hsearch_data env_htab = {
> +	.apply = env_check_apply,
> +};
> 
>  static uchar __env_get_char_spec(int index)
>  {
> @@ -183,7 +185,7 @@ void set_default_env(const char *s)
> 
>  	if (himport_r(&env_htab, (char *)default_environment,
>  			sizeof(default_environment), '\0', 0,
> -			0, NULL) == 0)
> +			0, NULL, 0 /* do_apply */) == 0)

Please replace that 0 /*...*/ stuff with simple NULL

>  		error("Environment import failed: errno = %d\n", errno);
> 
>  	gd->flags |= GD_FLG_ENV_READY;
> @@ -209,7 +211,7 @@ int env_import(const char *buf, int check)
>  	}
> 
>  	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
> -			0, NULL)) {
> +			0, NULL, 0 /* do_apply */)) {

DTTO, otherwise looks fine.

>  		gd->flags |= GD_FLG_ENV_READY;
>  		return 1;
>  	}
> diff --git a/include/environment.h b/include/environment.h
> index 1ef44f3..236e179 100644
> --- a/include/environment.h
> +++ b/include/environment.h
> @@ -193,6 +193,15 @@ void set_default_env(const char *s);
>  /* Import from binary representation into hash table */
>  int env_import(const char *buf, int check);
> 
> +/*
> + * Check if variable "name" can be changed from oldval to newval,
> + * and if so, apply the changes (e.g. baudrate).
> + * When (flag & H_FORCE) is set, it does not print out any error
> + * message and forces overwriting of write-once variables.
> + */
> +int env_check_apply(const char *name, const char *oldval,
> +			const char *newval, int flag);
> +
>  #endif /* DO_DEPS_ONLY */
> 
>  #endif /* _ENVIRONMENT_H_ */
> diff --git a/include/search.h b/include/search.h
> index 94d75fc..721c8ac 100644
> --- a/include/search.h
> +++ b/include/search.h
> @@ -57,6 +57,16 @@ struct hsearch_data {
>  	struct _ENTRY *table;
>  	unsigned int size;
>  	unsigned int filled;
> +/*
> + * Callback function which will check whether the given change for
> variable + * "name" from "oldval" to "newval" may be applied or not, and
> possibly apply + * such change.
> + * When (flag & H_FORCE) is set, it shall not print out any error message
> and + * shall force overwriting of write-once variables.
> +.* Must return 0 for approval, 1 for denial.
> + */
> +	int (*apply)(const char *name, const char *oldval,
> +			const char *newval, int flag);
>  };
> 
>  /* Create a new hashing table which will at most contain NEL elements.  */
> @@ -97,10 +107,12 @@ extern ssize_t hexport_r(struct hsearch_data *__htab,
>  /*
>   * nvars: length of vars array
>   * vars: array of strings (variable names) to import (nvars == 0 means
> all) + * do_apply: whether to call callback function to check the new
> argument, + * and possibly apply changes (false means accept everything)
>   */
>  extern int himport_r(struct hsearch_data *__htab,
>  		     const char *__env, size_t __size, const char __sep,
> -		     int __flag, int nvars, char * const vars[]);
> +		     int __flag, int nvars, char * const vars[], int do_apply);
> 
>  /* 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 0610e86..6cfba56 100644
> --- a/lib/hashtable.c
> +++ b/lib/hashtable.c
> @@ -658,7 +658,7 @@ static int is_var_in_set(const char *name, int nvars,
> char * const vars[])
> 
>  int himport_r(struct hsearch_data *htab,
>  		const char *env, size_t size, const char sep, int flag,
> -		int nvars, char * const vars[])
> +		int nvars, char * const vars[], int do_apply)
>  {
>  	char *data, *sp, *dp, *name, *value;
> 
> @@ -772,6 +772,24 @@ int himport_r(struct hsearch_data *htab,
>  		e.key = name;
>  		e.data = value;
> 
> +		/* if there is an apply function, check what it has to say */
> +		if (do_apply && htab->apply != NULL) {
> +			debug("searching before calling cb function"
> +				" for  %s\n", name);
> +			/*
> +			 * Search for variable in existing env, so to pass
> +			 * its previous value to the apply callback
> +			 */
> +			hsearch_r(e, FIND, &rv, htab);
> +			debug("previous value was %s\n", rv ? rv->data : "");
> +			if (htab->apply(name, rv ? rv->data : NULL,
> +				value, flag)) {
> +				debug("callback function refused to set"
> +					" variable %s, skipping it!\n", name);
> +				continue;
> +			}
> +		}
> +
>  		hsearch_r(e, ENTER, &rv, htab);
>  		if (rv == NULL) {
>  			printf("himport_r: can't insert \"%s=%s\" into hash 
table\n",
Marek Vasut April 2, 2012, 7:01 p.m. UTC | #2
Dear Marek Vasut,

> Dear Gerlando Falauto,
> 
> > Change hashtable so that a callback function will decide whether a
> > variable can be overwritten, and possibly apply the changes.
> > 
> > So add a new field to struct hsearch_data:
> >  o "apply" callback function to check whether a variable can be
> >  
> >     overwritten, and possibly immediately apply the changes;
> >     when NULL, no check is performed.
> > 
> > And a new argument to himport_r():
> >  o "do_apply": whether to call the apply callback function
> > 
> > NOTE: This patch does not change the current behavior.
> > 
> > Signed-off-by: Gerlando Falauto <gerlando.falauto@keymile.com>
> > ---
> > 
> >  common/cmd_nvedit.c   |    3 ++-
> >  common/env_common.c   |    8 +++++---
> >  include/environment.h |    9 +++++++++
> >  include/search.h      |   14 +++++++++++++-
> >  lib/hashtable.c       |   20 +++++++++++++++++++-
> >  5 files changed, 48 insertions(+), 6 deletions(-)
> > 
> > diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> > index 59668a6..fc6165f 100644
> > --- a/common/cmd_nvedit.c
> > +++ b/common/cmd_nvedit.c
> > @@ -201,6 +201,7 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
> > 
> >   * environment variable, then (if successful) apply the changes to
> > 
> > internals so * to make them effective.  Code for this function was taken
> > out of * _do_env_set(), which now calls it instead.
> > + * Also called as a callback function by himport_r().
> > 
> >   * Returns 0 in case of success, 1 in case of failure.
> >   * When (flag & H_FORCE) is set, do not print out any error message and
> > 
> > force * overwriting of write-once variables.
> > @@ -931,7 +932,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
> > 
> >  	}
> >  	
> >  	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
> > 
> > -			0, NULL) == 0) {
> > +			0, NULL, 0 /* do_apply */) == 0) {
> > 
> >  		error("Environment import failed: errno = %d\n", errno);
> >  		return 1;
> >  	
> >  	}
> > 
> > diff --git a/common/env_common.c b/common/env_common.c
> > index a93c062..5558c1c 100644
> > --- a/common/env_common.c
> > +++ b/common/env_common.c
> > @@ -122,7 +122,9 @@ const uchar default_environment[] = {
> > 
> >  	"\0"
> >  
> >  };
> > 
> > -struct hsearch_data env_htab;
> > +struct hsearch_data env_htab = {
> > +	.apply = env_check_apply,
> > +};
> > 
> >  static uchar __env_get_char_spec(int index)
> >  {
> > 
> > @@ -183,7 +185,7 @@ void set_default_env(const char *s)
> > 
> >  	if (himport_r(&env_htab, (char *)default_environment,
> >  	
> >  			sizeof(default_environment), '\0', 0,
> > 
> > -			0, NULL) == 0)
> > +			0, NULL, 0 /* do_apply */) == 0)
> 
> Please replace that 0 /*...*/ stuff with simple NULL

Oh, it's integer ... so just remove the comment, sorry.

> 
> >  		error("Environment import failed: errno = %d\n", errno);
> >  	
> >  	gd->flags |= GD_FLG_ENV_READY;
> > 
> > @@ -209,7 +211,7 @@ int env_import(const char *buf, int check)
> > 
> >  	}
> >  	
> >  	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
> > 
> > -			0, NULL)) {
> > +			0, NULL, 0 /* do_apply */)) {
> 
> DTTO, otherwise looks fine.
> 
> >  		gd->flags |= GD_FLG_ENV_READY;
> >  		return 1;
> >  	
> >  	}
> > 
> > diff --git a/include/environment.h b/include/environment.h
> > index 1ef44f3..236e179 100644
> > --- a/include/environment.h
> > +++ b/include/environment.h
> > @@ -193,6 +193,15 @@ void set_default_env(const char *s);
> > 
> >  /* Import from binary representation into hash table */
> >  int env_import(const char *buf, int check);
> > 
> > +/*
> > + * Check if variable "name" can be changed from oldval to newval,
> > + * and if so, apply the changes (e.g. baudrate).
> > + * When (flag & H_FORCE) is set, it does not print out any error
> > + * message and forces overwriting of write-once variables.
> > + */
> > +int env_check_apply(const char *name, const char *oldval,
> > +			const char *newval, int flag);
> > +
> > 
> >  #endif /* DO_DEPS_ONLY */
> >  
> >  #endif /* _ENVIRONMENT_H_ */
> > 
> > diff --git a/include/search.h b/include/search.h
> > index 94d75fc..721c8ac 100644
> > --- a/include/search.h
> > +++ b/include/search.h
> > @@ -57,6 +57,16 @@ struct hsearch_data {
> > 
> >  	struct _ENTRY *table;
> >  	unsigned int size;
> >  	unsigned int filled;
> > 
> > +/*
> > + * Callback function which will check whether the given change for
> > variable + * "name" from "oldval" to "newval" may be applied or not, and
> > possibly apply + * such change.
> > + * When (flag & H_FORCE) is set, it shall not print out any error
> > message and + * shall force overwriting of write-once variables.
> > +.* Must return 0 for approval, 1 for denial.
> > + */
> > +	int (*apply)(const char *name, const char *oldval,
> > +			const char *newval, int flag);
> > 
> >  };
> >  
> >  /* Create a new hashing table which will at most contain NEL elements. 
> >  */
> > 
> > @@ -97,10 +107,12 @@ extern ssize_t hexport_r(struct hsearch_data
> > *__htab,
> > 
> >  /*
> >  
> >   * nvars: length of vars array
> >   * vars: array of strings (variable names) to import (nvars == 0 means
> > 
> > all) + * do_apply: whether to call callback function to check the new
> > argument, + * and possibly apply changes (false means accept everything)
> > 
> >   */
> >  
> >  extern int himport_r(struct hsearch_data *__htab,
> >  
> >  		     const char *__env, size_t __size, const char __sep,
> > 
> > -		     int __flag, int nvars, char * const vars[]);
> > +		     int __flag, int nvars, char * const vars[], int do_apply);
> > 
> >  /* 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 0610e86..6cfba56 100644
> > --- a/lib/hashtable.c
> > +++ b/lib/hashtable.c
> > @@ -658,7 +658,7 @@ static int is_var_in_set(const char *name, int nvars,
> > char * const vars[])
> > 
> >  int himport_r(struct hsearch_data *htab,
> >  
> >  		const char *env, size_t size, const char sep, int flag,
> > 
> > -		int nvars, char * const vars[])
> > +		int nvars, char * const vars[], int do_apply)
> > 
> >  {
> >  
> >  	char *data, *sp, *dp, *name, *value;
> > 
> > @@ -772,6 +772,24 @@ int himport_r(struct hsearch_data *htab,
> > 
> >  		e.key = name;
> >  		e.data = value;
> > 
> > +		/* if there is an apply function, check what it has to say */
> > +		if (do_apply && htab->apply != NULL) {
> > +			debug("searching before calling cb function"
> > +				" for  %s\n", name);
> > +			/*
> > +			 * Search for variable in existing env, so to pass
> > +			 * its previous value to the apply callback
> > +			 */
> > +			hsearch_r(e, FIND, &rv, htab);
> > +			debug("previous value was %s\n", rv ? rv->data : "");
> > +			if (htab->apply(name, rv ? rv->data : NULL,
> > +				value, flag)) {
> > +				debug("callback function refused to set"
> > +					" variable %s, skipping it!\n", name);
> > +				continue;
> > +			}
> > +		}
> > +
> > 
> >  		hsearch_r(e, ENTER, &rv, htab);
> >  		if (rv == NULL) {
> >  		
> >  			printf("himport_r: can't insert \"%s=%s\" into hash
> 
> table\n",
Gerlando Falauto April 2, 2012, 8:44 p.m. UTC | #3
On 04/02/2012 09:01 PM, Marek Vasut wrote:
> Dear Marek Vasut,
>
>> Dear Gerlando Falauto,
>>
>>> Change hashtable so that a callback function will decide whether a
>>> variable can be overwritten, and possibly apply the changes.
>>>
>>> So add a new field to struct hsearch_data:
>>>   o "apply" callback function to check whether a variable can be
>>>
>>>      overwritten, and possibly immediately apply the changes;
>>>      when NULL, no check is performed.
>>>
>>> And a new argument to himport_r():
>>>   o "do_apply": whether to call the apply callback function
>>>
>>> NOTE: This patch does not change the current behavior.
>>>
>>> Signed-off-by: Gerlando Falauto<gerlando.falauto@keymile.com>
>>> ---
>>>
>>>   common/cmd_nvedit.c   |    3 ++-
>>>   common/env_common.c   |    8 +++++---
>>>   include/environment.h |    9 +++++++++
>>>   include/search.h      |   14 +++++++++++++-
>>>   lib/hashtable.c       |   20 +++++++++++++++++++-
>>>   5 files changed, 48 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
>>> index 59668a6..fc6165f 100644
>>> --- a/common/cmd_nvedit.c
>>> +++ b/common/cmd_nvedit.c
>>> @@ -201,6 +201,7 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
>>>
>>>    * environment variable, then (if successful) apply the changes to
>>>
>>> internals so * to make them effective.  Code for this function was taken
>>> out of * _do_env_set(), which now calls it instead.
>>> + * Also called as a callback function by himport_r().
>>>
>>>    * Returns 0 in case of success, 1 in case of failure.
>>>    * When (flag&  H_FORCE) is set, do not print out any error message and
>>>
>>> force * overwriting of write-once variables.
>>> @@ -931,7 +932,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,
>>>
>>>   	}
>>>   	
>>>   	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
>>>
>>> -			0, NULL) == 0) {
>>> +			0, NULL, 0 /* do_apply */) == 0) {
>>>
>>>   		error("Environment import failed: errno = %d\n", errno);
>>>   		return 1;
>>>   	
>>>   	}
>>>
>>> diff --git a/common/env_common.c b/common/env_common.c
>>> index a93c062..5558c1c 100644
>>> --- a/common/env_common.c
>>> +++ b/common/env_common.c
>>> @@ -122,7 +122,9 @@ const uchar default_environment[] = {
>>>
>>>   	"\0"
>>>
>>>   };
>>>
>>> -struct hsearch_data env_htab;
>>> +struct hsearch_data env_htab = {
>>> +	.apply = env_check_apply,
>>> +};
>>>
>>>   static uchar __env_get_char_spec(int index)
>>>   {
>>>
>>> @@ -183,7 +185,7 @@ void set_default_env(const char *s)
>>>
>>>   	if (himport_r(&env_htab, (char *)default_environment,
>>>   	
>>>   			sizeof(default_environment), '\0', 0,
>>>
>>> -			0, NULL) == 0)
>>> +			0, NULL, 0 /* do_apply */) == 0)
>>
>> Please replace that 0 /*...*/ stuff with simple NULL
>
> Oh, it's integer ... so just remove the comment, sorry.

The comment would have been a way to shed some light on one of the 8 
arguments the function takes.
But if it's against some coding style guideline, I'll simply remove it.

>
>>
>>>   		error("Environment import failed: errno = %d\n", errno);
>>>   	
>>>   	gd->flags |= GD_FLG_ENV_READY;
>>>
>>> @@ -209,7 +211,7 @@ int env_import(const char *buf, int check)
>>>
>>>   	}
>>>   	
>>>   	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
>>>
>>> -			0, NULL)) {
>>> +			0, NULL, 0 /* do_apply */)) {
>>
>> DTTO, otherwise looks fine.
>>
>>>   		gd->flags |= GD_FLG_ENV_READY;
>>>   		return 1;
>>>   	
>>>   	}
>>>
>>> diff --git a/include/environment.h b/include/environment.h
>>> index 1ef44f3..236e179 100644
>>> --- a/include/environment.h
>>> +++ b/include/environment.h
>>> @@ -193,6 +193,15 @@ void set_default_env(const char *s);
>>>
>>>   /* Import from binary representation into hash table */
>>>   int env_import(const char *buf, int check);
>>>
>>> +/*
>>> + * Check if variable "name" can be changed from oldval to newval,
>>> + * and if so, apply the changes (e.g. baudrate).
>>> + * When (flag&  H_FORCE) is set, it does not print out any error
>>> + * message and forces overwriting of write-once variables.
>>> + */
>>> +int env_check_apply(const char *name, const char *oldval,
>>> +			const char *newval, int flag);
>>> +
>>>
>>>   #endif /* DO_DEPS_ONLY */
>>>
>>>   #endif /* _ENVIRONMENT_H_ */
>>>
>>> diff --git a/include/search.h b/include/search.h
>>> index 94d75fc..721c8ac 100644
>>> --- a/include/search.h
>>> +++ b/include/search.h
>>> @@ -57,6 +57,16 @@ struct hsearch_data {
>>>
>>>   	struct _ENTRY *table;
>>>   	unsigned int size;
>>>   	unsigned int filled;
>>>
>>> +/*
>>> + * Callback function which will check whether the given change for
>>> variable + * "name" from "oldval" to "newval" may be applied or not, and
>>> possibly apply + * such change.
>>> + * When (flag&  H_FORCE) is set, it shall not print out any error
>>> message and + * shall force overwriting of write-once variables.
>>> +.* Must return 0 for approval, 1 for denial.
>>> + */
>>> +	int (*apply)(const char *name, const char *oldval,
>>> +			const char *newval, int flag);
>>>
>>>   };
>>>
>>>   /* Create a new hashing table which will at most contain NEL elements.
>>>   */
>>>
>>> @@ -97,10 +107,12 @@ extern ssize_t hexport_r(struct hsearch_data
>>> *__htab,
>>>
>>>   /*
>>>
>>>    * nvars: length of vars array
>>>    * vars: array of strings (variable names) to import (nvars == 0 means
>>>
>>> all) + * do_apply: whether to call callback function to check the new
>>> argument, + * and possibly apply changes (false means accept everything)
>>>
>>>    */
>>>
>>>   extern int himport_r(struct hsearch_data *__htab,
>>>
>>>   		     const char *__env, size_t __size, const char __sep,
>>>
>>> -		     int __flag, int nvars, char * const vars[]);
>>> +		     int __flag, int nvars, char * const vars[], int do_apply);
>>>
>>>   /* 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 0610e86..6cfba56 100644
>>> --- a/lib/hashtable.c
>>> +++ b/lib/hashtable.c
>>> @@ -658,7 +658,7 @@ static int is_var_in_set(const char *name, int nvars,
>>> char * const vars[])
>>>
>>>   int himport_r(struct hsearch_data *htab,
>>>
>>>   		const char *env, size_t size, const char sep, int flag,
>>>
>>> -		int nvars, char * const vars[])
>>> +		int nvars, char * const vars[], int do_apply)
>>>
>>>   {
>>>
>>>   	char *data, *sp, *dp, *name, *value;
>>>
>>> @@ -772,6 +772,24 @@ int himport_r(struct hsearch_data *htab,
>>>
>>>   		e.key = name;
>>>   		e.data = value;
>>>
>>> +		/* if there is an apply function, check what it has to say */
>>> +		if (do_apply&&  htab->apply != NULL) {
>>> +			debug("searching before calling cb function"
>>> +				" for  %s\n", name);
>>> +			/*
>>> +			 * Search for variable in existing env, so to pass
>>> +			 * its previous value to the apply callback
>>> +			 */
>>> +			hsearch_r(e, FIND,&rv, htab);
>>> +			debug("previous value was %s\n", rv ? rv->data : "");
>>> +			if (htab->apply(name, rv ? rv->data : NULL,
>>> +				value, flag)) {
>>> +				debug("callback function refused to set"
>>> +					" variable %s, skipping it!\n", name);
>>> +				continue;
>>> +			}
>>> +		}
>>> +
>>>
>>>   		hsearch_r(e, ENTER,&rv, htab);
>>>   		if (rv == NULL) {
>>>   		
>>>   			printf("himport_r: can't insert \"%s=%s\" into hash
>>
>> table\n",
Marek Vasut April 2, 2012, 8:51 p.m. UTC | #4
Dear Gerlando Falauto,

> On 04/02/2012 09:01 PM, Marek Vasut wrote:
> > Dear Marek Vasut,
> > 
> >> Dear Gerlando Falauto,
> >> 
> >>> Change hashtable so that a callback function will decide whether a
> >>> variable can be overwritten, and possibly apply the changes.
> >>> 
> >>> So add a new field to struct hsearch_data:
> >>>   o "apply" callback function to check whether a variable can be
> >>>   
> >>>      overwritten, and possibly immediately apply the changes;
> >>>      when NULL, no check is performed.
> >>> 
> >>> And a new argument to himport_r():
> >>>   o "do_apply": whether to call the apply callback function
> >>> 
> >>> NOTE: This patch does not change the current behavior.
> >>> 
> >>> Signed-off-by: Gerlando Falauto<gerlando.falauto@keymile.com>
> >>> ---
> >>> 
> >>>   common/cmd_nvedit.c   |    3 ++-
> >>>   common/env_common.c   |    8 +++++---
> >>>   include/environment.h |    9 +++++++++
> >>>   include/search.h      |   14 +++++++++++++-
> >>>   lib/hashtable.c       |   20 +++++++++++++++++++-
> >>>   5 files changed, 48 insertions(+), 6 deletions(-)
> >>> 
> >>> diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
> >>> index 59668a6..fc6165f 100644
> >>> --- a/common/cmd_nvedit.c
> >>> +++ b/common/cmd_nvedit.c
> >>> @@ -201,6 +201,7 @@ static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
> >>> 
> >>>    * environment variable, then (if successful) apply the changes to
> >>> 
> >>> internals so * to make them effective.  Code for this function was
> >>> taken out of * _do_env_set(), which now calls it instead.
> >>> + * Also called as a callback function by himport_r().
> >>> 
> >>>    * Returns 0 in case of success, 1 in case of failure.
> >>>    * When (flag&  H_FORCE) is set, do not print out any error message
> >>>    and
> >>> 
> >>> force * overwriting of write-once variables.
> >>> @@ -931,7 +932,7 @@ static int do_env_import(cmd_tbl_t *cmdtp, int
> >>> flag,
> >>> 
> >>>   	}
> >>>   	
> >>>   	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
> >>> 
> >>> -			0, NULL) == 0) {
> >>> +			0, NULL, 0 /* do_apply */) == 0) {
> >>> 
> >>>   		error("Environment import failed: errno = %d\n", errno);
> >>>   		return 1;
> >>>   	
> >>>   	}
> >>> 
> >>> diff --git a/common/env_common.c b/common/env_common.c
> >>> index a93c062..5558c1c 100644
> >>> --- a/common/env_common.c
> >>> +++ b/common/env_common.c
> >>> @@ -122,7 +122,9 @@ const uchar default_environment[] = {
> >>> 
> >>>   	"\0"
> >>>   
> >>>   };
> >>> 
> >>> -struct hsearch_data env_htab;
> >>> +struct hsearch_data env_htab = {
> >>> +	.apply = env_check_apply,
> >>> +};
> >>> 
> >>>   static uchar __env_get_char_spec(int index)
> >>>   {
> >>> 
> >>> @@ -183,7 +185,7 @@ void set_default_env(const char *s)
> >>> 
> >>>   	if (himport_r(&env_htab, (char *)default_environment,
> >>>   	
> >>>   			sizeof(default_environment), '\0', 0,
> >>> 
> >>> -			0, NULL) == 0)
> >>> +			0, NULL, 0 /* do_apply */) == 0)
> >> 
> >> Please replace that 0 /*...*/ stuff with simple NULL
> > 
> > Oh, it's integer ... so just remove the comment, sorry.
> 
> The comment would have been a way to shed some light on one of the 8
> arguments the function takes.
> But if it's against some coding style guideline, I'll simply remove it.

Not really, I don't really care ;-)

> 
> >>>   		error("Environment import failed: errno = %d\n", errno);
> >>>   	
> >>>   	gd->flags |= GD_FLG_ENV_READY;
> >>> 
> >>> @@ -209,7 +211,7 @@ int env_import(const char *buf, int check)
> >>> 
> >>>   	}
> >>>   	
> >>>   	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
> >>> 
> >>> -			0, NULL)) {
> >>> +			0, NULL, 0 /* do_apply */)) {
> >> 
> >> DTTO, otherwise looks fine.
> >> 
> >>>   		gd->flags |= GD_FLG_ENV_READY;
> >>>   		return 1;
> >>>   	
> >>>   	}
> >>> 
> >>> diff --git a/include/environment.h b/include/environment.h
> >>> index 1ef44f3..236e179 100644
> >>> --- a/include/environment.h
> >>> +++ b/include/environment.h
> >>> @@ -193,6 +193,15 @@ void set_default_env(const char *s);
> >>> 
> >>>   /* Import from binary representation into hash table */
> >>>   int env_import(const char *buf, int check);
> >>> 
> >>> +/*
> >>> + * Check if variable "name" can be changed from oldval to newval,
> >>> + * and if so, apply the changes (e.g. baudrate).
> >>> + * When (flag&  H_FORCE) is set, it does not print out any error
> >>> + * message and forces overwriting of write-once variables.
> >>> + */
> >>> +int env_check_apply(const char *name, const char *oldval,
> >>> +			const char *newval, int flag);
> >>> +
> >>> 
> >>>   #endif /* DO_DEPS_ONLY */
> >>>   
> >>>   #endif /* _ENVIRONMENT_H_ */
> >>> 
> >>> diff --git a/include/search.h b/include/search.h
> >>> index 94d75fc..721c8ac 100644
> >>> --- a/include/search.h
> >>> +++ b/include/search.h
> >>> @@ -57,6 +57,16 @@ struct hsearch_data {
> >>> 
> >>>   	struct _ENTRY *table;
> >>>   	unsigned int size;
> >>>   	unsigned int filled;
> >>> 
> >>> +/*
> >>> + * Callback function which will check whether the given change for
> >>> variable + * "name" from "oldval" to "newval" may be applied or not,
> >>> and possibly apply + * such change.
> >>> + * When (flag&  H_FORCE) is set, it shall not print out any error
> >>> message and + * shall force overwriting of write-once variables.
> >>> +.* Must return 0 for approval, 1 for denial.
> >>> + */
> >>> +	int (*apply)(const char *name, const char *oldval,
> >>> +			const char *newval, int flag);
> >>> 
> >>>   };
> >>>   
> >>>   /* Create a new hashing table which will at most contain NEL
> >>>   elements. */
> >>> 
> >>> @@ -97,10 +107,12 @@ extern ssize_t hexport_r(struct hsearch_data
> >>> *__htab,
> >>> 
> >>>   /*
> >>>   
> >>>    * nvars: length of vars array
> >>>    * vars: array of strings (variable names) to import (nvars == 0
> >>>    means
> >>> 
> >>> all) + * do_apply: whether to call callback function to check the new
> >>> argument, + * and possibly apply changes (false means accept
> >>> everything)
> >>> 
> >>>    */
> >>>   
> >>>   extern int himport_r(struct hsearch_data *__htab,
> >>>   
> >>>   		     const char *__env, size_t __size, const char __sep,
> >>> 
> >>> -		     int __flag, int nvars, char * const vars[]);
> >>> +		     int __flag, int nvars, char * const vars[], int do_apply);
> >>> 
> >>>   /* 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 0610e86..6cfba56 100644
> >>> --- a/lib/hashtable.c
> >>> +++ b/lib/hashtable.c
> >>> @@ -658,7 +658,7 @@ static int is_var_in_set(const char *name, int
> >>> nvars, char * const vars[])
> >>> 
> >>>   int himport_r(struct hsearch_data *htab,
> >>>   
> >>>   		const char *env, size_t size, const char sep, int flag,
> >>> 
> >>> -		int nvars, char * const vars[])
> >>> +		int nvars, char * const vars[], int do_apply)
> >>> 
> >>>   {
> >>>   
> >>>   	char *data, *sp, *dp, *name, *value;
> >>> 
> >>> @@ -772,6 +772,24 @@ int himport_r(struct hsearch_data *htab,
> >>> 
> >>>   		e.key = name;
> >>>   		e.data = value;
> >>> 
> >>> +		/* if there is an apply function, check what it has to say */
> >>> +		if (do_apply&&  htab->apply != NULL) {
> >>> +			debug("searching before calling cb function"
> >>> +				" for  %s\n", name);
> >>> +			/*
> >>> +			 * Search for variable in existing env, so to pass
> >>> +			 * its previous value to the apply callback
> >>> +			 */
> >>> +			hsearch_r(e, FIND,&rv, htab);
> >>> +			debug("previous value was %s\n", rv ? rv->data : "");
> >>> +			if (htab->apply(name, rv ? rv->data : NULL,
> >>> +				value, flag)) {
> >>> +				debug("callback function refused to set"
> >>> +					" variable %s, skipping it!\n", name);
> >>> +				continue;
> >>> +			}
> >>> +		}
> >>> +
> >>> 
> >>>   		hsearch_r(e, ENTER,&rv, htab);
> >>>   		if (rv == NULL) {
> >>>   		
> >>>   			printf("himport_r: can't insert \"%s=%s\" into 
hash
> >> 
> >> table\n",
diff mbox

Patch

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index 59668a6..fc6165f 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -201,6 +201,7 @@  static int do_env_grep(cmd_tbl_t *cmdtp, int flag,
  * environment variable, then (if successful) apply the changes to internals so
  * to make them effective.  Code for this function was taken out of
  * _do_env_set(), which now calls it instead.
+ * Also called as a callback function by himport_r().
  * Returns 0 in case of success, 1 in case of failure.
  * When (flag & H_FORCE) is set, do not print out any error message and force
  * overwriting of write-once variables.
@@ -931,7 +932,7 @@  static int do_env_import(cmd_tbl_t *cmdtp, int flag,
 	}
 
 	if (himport_r(&env_htab, addr, size, sep, del ? 0 : H_NOCLEAR,
-			0, NULL) == 0) {
+			0, NULL, 0 /* do_apply */) == 0) {
 		error("Environment import failed: errno = %d\n", errno);
 		return 1;
 	}
diff --git a/common/env_common.c b/common/env_common.c
index a93c062..5558c1c 100644
--- a/common/env_common.c
+++ b/common/env_common.c
@@ -122,7 +122,9 @@  const uchar default_environment[] = {
 	"\0"
 };
 
-struct hsearch_data env_htab;
+struct hsearch_data env_htab = {
+	.apply = env_check_apply,
+};
 
 static uchar __env_get_char_spec(int index)
 {
@@ -183,7 +185,7 @@  void set_default_env(const char *s)
 
 	if (himport_r(&env_htab, (char *)default_environment,
 			sizeof(default_environment), '\0', 0,
-			0, NULL) == 0)
+			0, NULL, 0 /* do_apply */) == 0)
 		error("Environment import failed: errno = %d\n", errno);
 
 	gd->flags |= GD_FLG_ENV_READY;
@@ -209,7 +211,7 @@  int env_import(const char *buf, int check)
 	}
 
 	if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0,
-			0, NULL)) {
+			0, NULL, 0 /* do_apply */)) {
 		gd->flags |= GD_FLG_ENV_READY;
 		return 1;
 	}
diff --git a/include/environment.h b/include/environment.h
index 1ef44f3..236e179 100644
--- a/include/environment.h
+++ b/include/environment.h
@@ -193,6 +193,15 @@  void set_default_env(const char *s);
 /* Import from binary representation into hash table */
 int env_import(const char *buf, int check);
 
+/*
+ * Check if variable "name" can be changed from oldval to newval,
+ * and if so, apply the changes (e.g. baudrate).
+ * When (flag & H_FORCE) is set, it does not print out any error
+ * message and forces overwriting of write-once variables.
+ */
+int env_check_apply(const char *name, const char *oldval,
+			const char *newval, int flag);
+
 #endif /* DO_DEPS_ONLY */
 
 #endif /* _ENVIRONMENT_H_ */
diff --git a/include/search.h b/include/search.h
index 94d75fc..721c8ac 100644
--- a/include/search.h
+++ b/include/search.h
@@ -57,6 +57,16 @@  struct hsearch_data {
 	struct _ENTRY *table;
 	unsigned int size;
 	unsigned int filled;
+/*
+ * Callback function which will check whether the given change for variable
+ * "name" from "oldval" to "newval" may be applied or not, and possibly apply
+ * such change.
+ * When (flag & H_FORCE) is set, it shall not print out any error message and
+ * shall force overwriting of write-once variables.
+.* Must return 0 for approval, 1 for denial.
+ */
+	int (*apply)(const char *name, const char *oldval,
+			const char *newval, int flag);
 };
 
 /* Create a new hashing table which will at most contain NEL elements.  */
@@ -97,10 +107,12 @@  extern ssize_t hexport_r(struct hsearch_data *__htab,
 /*
  * nvars: length of vars array
  * vars: array of strings (variable names) to import (nvars == 0 means all)
+ * do_apply: whether to call callback function to check the new argument,
+ * and possibly apply changes (false means accept everything)
  */
 extern int himport_r(struct hsearch_data *__htab,
 		     const char *__env, size_t __size, const char __sep,
-		     int __flag, int nvars, char * const vars[]);
+		     int __flag, int nvars, char * const vars[], int do_apply);
 
 /* 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 0610e86..6cfba56 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -658,7 +658,7 @@  static int is_var_in_set(const char *name, int nvars, char * const vars[])
 
 int himport_r(struct hsearch_data *htab,
 		const char *env, size_t size, const char sep, int flag,
-		int nvars, char * const vars[])
+		int nvars, char * const vars[], int do_apply)
 {
 	char *data, *sp, *dp, *name, *value;
 
@@ -772,6 +772,24 @@  int himport_r(struct hsearch_data *htab,
 		e.key = name;
 		e.data = value;
 
+		/* if there is an apply function, check what it has to say */
+		if (do_apply && htab->apply != NULL) {
+			debug("searching before calling cb function"
+				" for  %s\n", name);
+			/*
+			 * Search for variable in existing env, so to pass
+			 * its previous value to the apply callback
+			 */
+			hsearch_r(e, FIND, &rv, htab);
+			debug("previous value was %s\n", rv ? rv->data : "");
+			if (htab->apply(name, rv ? rv->data : NULL,
+				value, flag)) {
+				debug("callback function refused to set"
+					" variable %s, skipping it!\n", name);
+				continue;
+			}
+		}
+
 		hsearch_r(e, ENTER, &rv, htab);
 		if (rv == NULL) {
 			printf("himport_r: can't insert \"%s=%s\" into hash table\n",