diff mbox

[U-Boot,06/11] env: Allow env_attr_walk to pass a priv * to callback

Message ID 1429653771-11676-7-git-send-email-joe.hershberger@ni.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Joe Hershberger April 21, 2015, 10:02 p.m. UTC 
In some cases it can be helpful to have context in the callback about
the calling situation. This is needed for following patches.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 common/cmd_nvedit.c   | 10 ++++++----
 common/env_attr.c     | 15 ++++++++++-----
 common/env_callback.c |  6 +++---
 common/env_flags.c    |  6 +++---
 include/env_attr.h    | 10 +++++-----
 5 files changed, 27 insertions(+), 20 deletions(-)

Comments

Joe Hershberger April 21, 2015, 10:20 p.m. UTC | #1 
Hi All,

On Tue, Apr 21, 2015 at 5:02 PM, Joe Hershberger <joe.hershberger@ni.com> wrote:
> In some cases it can be helpful to have context in the callback about
> the calling situation. This is needed for following patches.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---

8<--snip-->8

> diff --git a/common/env_attr.c b/common/env_attr.c
> index d266142..f0bf504 100644
> --- a/common/env_attr.c
> +++ b/common/env_attr.c
> @@ -26,7 +26,8 @@
>   *     list = entry[,list]
>   */
>  int env_attr_walk(const char *attr_list,
> -       int (*callback)(const char *name, const char *attributes))
> +       int (*callback)(const char *name, const char *attributes, void *priv),
> +       void *priv)
>  {
>         const char *entry, *entry_end;
>         char *name, *attributes;
> @@ -93,7 +94,7 @@ int env_attr_walk(const char *attr_list,
>                         if (strlen(name) != 0) {
>                                 int retval = 0;
>
> -                               retval = callback(name, attributes);
> +                               retval = callback(name, attributes, priv);
>                                 if (retval) {
>                                         free(entry_cpy);
>                                         return retval;
> @@ -120,8 +121,11 @@ static int reverse_name_search(const char *searched, const char *search_for,
>         if (result)
>                 *result = NULL;
>
> -       if (*search_for == '\0')
> -               return (char *)searched;
> +       if (*search_for == '\0') {
> +               if (result)
> +                       *result = searched;
> +               return strlen(searched);
> +       }

I noticed shortly after sending this that this hunk belongs in the
previous patch.

>
>         for (;;) {
>                 const char *match = strstr(cur_searched, search_for);
> @@ -153,7 +157,8 @@ static int reverse_name_search(const char *searched, const char *search_for,
>                     *nextch != '\0')
>                         continue;
>
> -               *result = match;
> +               if (result)
> +                       *result = match;

As does this hunk.

>                 result_size = strlen(search_for);
>         }
>

8<--snip-->8

My apologies... I'll resend after any other comments.  Please do not
apply as is, Tom.

Thanks,
-Joe
Simon Glass April 24, 2015, 4:34 a.m. UTC | #2 
On 21 April 2015 at 16:20, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi All,
>
> On Tue, Apr 21, 2015 at 5:02 PM, Joe Hershberger <joe.hershberger@ni.com> wrote:
>> In some cases it can be helpful to have context in the callback about
>> the calling situation. This is needed for following patches.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>> ---
>
> 8<--snip-->8
>
>> diff --git a/common/env_attr.c b/common/env_attr.c
>> index d266142..f0bf504 100644
>> --- a/common/env_attr.c
>> +++ b/common/env_attr.c
>> @@ -26,7 +26,8 @@
>>   *     list = entry[,list]
>>   */
>>  int env_attr_walk(const char *attr_list,
>> -       int (*callback)(const char *name, const char *attributes))
>> +       int (*callback)(const char *name, const char *attributes, void *priv),
>> +       void *priv)
>>  {
>>         const char *entry, *entry_end;
>>         char *name, *attributes;
>> @@ -93,7 +94,7 @@ int env_attr_walk(const char *attr_list,
>>                         if (strlen(name) != 0) {
>>                                 int retval = 0;
>>
>> -                               retval = callback(name, attributes);
>> +                               retval = callback(name, attributes, priv);
>>                                 if (retval) {
>>                                         free(entry_cpy);
>>                                         return retval;
>> @@ -120,8 +121,11 @@ static int reverse_name_search(const char *searched, const char *search_for,
>>         if (result)
>>                 *result = NULL;
>>
>> -       if (*search_for == '\0')
>> -               return (char *)searched;
>> +       if (*search_for == '\0') {
>> +               if (result)
>> +                       *result = searched;
>> +               return strlen(searched);
>> +       }
>
> I noticed shortly after sending this that this hunk belongs in the
> previous patch.
>
>>
>>         for (;;) {
>>                 const char *match = strstr(cur_searched, search_for);
>> @@ -153,7 +157,8 @@ static int reverse_name_search(const char *searched, const char *search_for,
>>                     *nextch != '\0')
>>                         continue;
>>
>> -               *result = match;
>> +               if (result)
>> +                       *result = match;
>
> As does this hunk.
>
>>                 result_size = strlen(search_for);
>>         }
>>
>
> 8<--snip-->8
>
> My apologies... I'll resend after any other comments.  Please do not
> apply as is, Tom.
>
> Thanks,
> -Joe

Reviewed-by: Simon Glass <sjg@chromium.org>
diff mbox

Patch

diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c
index be792ae..6ca5a2e 100644
--- a/common/cmd_nvedit.c
+++ b/common/cmd_nvedit.c
@@ -427,7 +427,8 @@  int do_env_ask(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 #endif
 
 #if defined(CONFIG_CMD_ENV_CALLBACK)
-static int print_static_binding(const char *var_name, const char *callback_name)
+static int print_static_binding(const char *var_name, const char *callback_name,
+				void *priv)
 {
 	printf("\t%-20s %-20s\n", var_name, callback_name);
 
@@ -489,7 +490,7 @@  int do_env_callback(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	puts("Static callback bindings:\n");
 	printf("\t%-20s %-20s\n", "Variable Name", "Callback Name");
 	printf("\t%-20s %-20s\n", "-------------", "-------------");
-	env_attr_walk(ENV_CALLBACK_LIST_STATIC, print_static_binding);
+	env_attr_walk(ENV_CALLBACK_LIST_STATIC, print_static_binding, NULL);
 	puts("\n");
 
 	/* walk through each variable and print the callback if it has one */
@@ -502,7 +503,8 @@  int do_env_callback(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 #endif
 
 #if defined(CONFIG_CMD_ENV_FLAGS)
-static int print_static_flags(const char *var_name, const char *flags)
+static int print_static_flags(const char *var_name, const char *flags,
+			      void *priv)
 {
 	enum env_flags_vartype type = env_flags_parse_vartype(flags);
 	enum env_flags_varaccess access = env_flags_parse_varaccess(flags);
@@ -559,7 +561,7 @@  int do_env_flags(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		"Variable Access");
 	printf("\t%-20s %-20s %-20s\n", "-------------", "-------------",
 		"---------------");
-	env_attr_walk(ENV_FLAGS_LIST_STATIC, print_static_flags);
+	env_attr_walk(ENV_FLAGS_LIST_STATIC, print_static_flags, NULL);
 	puts("\n");
 
 	/* walk through each variable and print the flags if non-default */
diff --git a/common/env_attr.c b/common/env_attr.c
index d266142..f0bf504 100644
--- a/common/env_attr.c
+++ b/common/env_attr.c
@@ -26,7 +26,8 @@ 
  *	list = entry[,list]
  */
 int env_attr_walk(const char *attr_list,
-	int (*callback)(const char *name, const char *attributes))
+	int (*callback)(const char *name, const char *attributes, void *priv),
+	void *priv)
 {
 	const char *entry, *entry_end;
 	char *name, *attributes;
@@ -93,7 +94,7 @@  int env_attr_walk(const char *attr_list,
 			if (strlen(name) != 0) {
 				int retval = 0;
 
-				retval = callback(name, attributes);
+				retval = callback(name, attributes, priv);
 				if (retval) {
 					free(entry_cpy);
 					return retval;
@@ -120,8 +121,11 @@  static int reverse_name_search(const char *searched, const char *search_for,
 	if (result)
 		*result = NULL;
 
-	if (*search_for == '\0')
-		return (char *)searched;
+	if (*search_for == '\0') {
+		if (result)
+			*result = searched;
+		return strlen(searched);
+	}
 
 	for (;;) {
 		const char *match = strstr(cur_searched, search_for);
@@ -153,7 +157,8 @@  static int reverse_name_search(const char *searched, const char *search_for,
 		    *nextch != '\0')
 			continue;
 
-		*result = match;
+		if (result)
+			*result = match;
 		result_size = strlen(search_for);
 	}
 
diff --git a/common/env_callback.c b/common/env_callback.c
index d03fa03..f4d3dbd 100644
--- a/common/env_callback.c
+++ b/common/env_callback.c
@@ -90,7 +90,7 @@  static int clear_callback(ENTRY *entry)
 /*
  * Call for each element in the list that associates variables to callbacks
  */
-static int set_callback(const char *name, const char *value)
+static int set_callback(const char *name, const char *value, void *priv)
 {
 	ENTRY e, *ep;
 	struct env_clbk_tbl *clbkp;
@@ -126,9 +126,9 @@  static int on_callbacks(const char *name, const char *value, enum env_op op,
 	hwalk_r(&env_htab, clear_callback);
 
 	/* configure any static callback bindings */
-	env_attr_walk(ENV_CALLBACK_LIST_STATIC, set_callback);
+	env_attr_walk(ENV_CALLBACK_LIST_STATIC, set_callback, NULL);
 	/* configure any dynamic callback bindings */
-	env_attr_walk(value, set_callback);
+	env_attr_walk(value, set_callback, NULL);
 
 	return 0;
 }
diff --git a/common/env_flags.c b/common/env_flags.c
index 985f92e..5189f5b 100644
--- a/common/env_flags.c
+++ b/common/env_flags.c
@@ -435,7 +435,7 @@  static int clear_flags(ENTRY *entry)
 /*
  * Call for each element in the list that defines flags for a variable
  */
-static int set_flags(const char *name, const char *value)
+static int set_flags(const char *name, const char *value, void *priv)
 {
 	ENTRY e, *ep;
 
@@ -463,9 +463,9 @@  static int on_flags(const char *name, const char *value, enum env_op op,
 	hwalk_r(&env_htab, clear_flags);
 
 	/* configure any static flags */
-	env_attr_walk(ENV_FLAGS_LIST_STATIC, set_flags);
+	env_attr_walk(ENV_FLAGS_LIST_STATIC, set_flags, NULL);
 	/* configure any dynamic flags */
-	env_attr_walk(value, set_flags);
+	env_attr_walk(value, set_flags, NULL);
 
 	return 0;
 }
diff --git a/include/env_attr.h b/include/env_attr.h
index b82fec9..7bfb7f3 100644
--- a/include/env_attr.h
+++ b/include/env_attr.h
@@ -16,13 +16,14 @@ 
  *	attributes = [^,:\s]*
  *	entry = name[:attributes]
  *	list = entry[,list]
- * It will call the "callback" function with the "name" and attribute as "value"
+ * It will call the "callback" function with the "name" and "attributes"
  * The callback may return a non-0 to abort the list walk.
  * This return value will be passed through to the caller.
  * 0 is returned on success.
  */
-extern int env_attr_walk(const char *attr_list,
-	int (*callback)(const char *name, const char *value));
+int env_attr_walk(const char *attr_list,
+	int (*callback)(const char *name, const char *attributes, void *priv),
+	void *priv);
 
 /*
  * env_attr_lookup takes as input an "attr_list" with the same form as above.
@@ -33,7 +34,6 @@  extern int env_attr_walk(const char *attr_list,
  * "attr_list" is NULL.
  * Returns 0 on success.
  */
-extern int env_attr_lookup(const char *attr_list, const char *name,
-	char *attributes);
+int env_attr_lookup(const char *attr_list, const char *name, char *attributes);
 
 #endif /* __ENV_ATTR_H__ */