Re: [U-Boot] [PATCH 05/11] env: Simplify the reverse_strstr() interface
Hi Joe, On 27 April 2015 at 13:31, Joe Hershberger joe.hershber...@gmail.com wrote: Hi Simon, On Mon, Apr 27, 2015 at 2:24 PM, Joe Hershberger joe.hershber...@gmail.com wrote: Hi Simon, On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass s...@chromium.org wrote: Hi Joe, On 21 April 2015 at 16:02, Joe Hershberger joe.hershber...@ni.com wrote: The logic to find the whole matching name was split needlessly between the reverse_strstr function and its caller. Fully contain it to make the interface for calling it more consistent. Signed-off-by: Joe Hershberger joe.hershber...@ni.com --- common/env_attr.c | 79 +-- 1 file changed, 41 insertions(+), 38 deletions(-) You could perhaps add some environment tests in test/ for this function, or access it through getenv(), etc. I'll look into adding some unit tests for the env stuff. I'd like to reuse a bit of the unit test code from DM tests... do you have any plans to generalize some of it? Or should I take a crack at some of it (that I want to reuse)? No plans, please go ahead! Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 05/11] env: Simplify the reverse_strstr() interface
Hi Simon, On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass s...@chromium.org wrote: Hi Joe, On 21 April 2015 at 16:02, Joe Hershberger joe.hershber...@ni.com wrote: The logic to find the whole matching name was split needlessly between the reverse_strstr function and its caller. Fully contain it to make the interface for calling it more consistent. Signed-off-by: Joe Hershberger joe.hershber...@ni.com --- common/env_attr.c | 79 +-- 1 file changed, 41 insertions(+), 38 deletions(-) You could perhaps add some environment tests in test/ for this function, or access it through getenv(), etc. I'll look into adding some unit tests for the env stuff. Cheers, -Joe ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 05/11] env: Simplify the reverse_strstr() interface
Hi Simon, On Mon, Apr 27, 2015 at 2:24 PM, Joe Hershberger joe.hershber...@gmail.com wrote: Hi Simon, On Thu, Apr 23, 2015 at 11:34 PM, Simon Glass s...@chromium.org wrote: Hi Joe, On 21 April 2015 at 16:02, Joe Hershberger joe.hershber...@ni.com wrote: The logic to find the whole matching name was split needlessly between the reverse_strstr function and its caller. Fully contain it to make the interface for calling it more consistent. Signed-off-by: Joe Hershberger joe.hershber...@ni.com --- common/env_attr.c | 79 +-- 1 file changed, 41 insertions(+), 38 deletions(-) You could perhaps add some environment tests in test/ for this function, or access it through getenv(), etc. I'll look into adding some unit tests for the env stuff. I'd like to reuse a bit of the unit test code from DM tests... do you have any plans to generalize some of it? Or should I take a crack at some of it (that I want to reuse)? Thanks, -Joe ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 05/11] env: Simplify the reverse_strstr() interface
Hi Joe,
On 21 April 2015 at 16:02, Joe Hershberger joe.hershber...@ni.com wrote:
The logic to find the whole matching name was split needlessly between
the reverse_strstr function and its caller. Fully contain it to make the
interface for calling it more consistent.
Signed-off-by: Joe Hershberger joe.hershber...@ni.com
---
common/env_attr.c | 79
+--
1 file changed, 41 insertions(+), 38 deletions(-)
You could perhaps add some environment tests in test/ for this
function, or access it through getenv(), etc.
diff --git a/common/env_attr.c b/common/env_attr.c
index e791f44..d266142 100644
--- a/common/env_attr.c
+++ b/common/env_attr.c
@@ -109,33 +109,55 @@ int env_attr_walk(const char *attr_list,
}
/*
- * Search for the last matching string in another string with the option to
- * start looking at a certain point (i.e. ignore anything beyond that point).
+ * Search for the last exactly matching name in an attribute list
*/
-static char *reverse_strstr(const char *searched, const char *search_for,
- const char *searched_start)
+static int reverse_name_search(const char *searched, const char *search_for,
+ const char **result)
{
- char *result = NULL;
+ int result_size = 0;
+ const char *cur_searched = searched;
+
+ if (result)
+ *result = NULL;
if (*search_for == '\0')
return (char *)searched;
for (;;) {
- char *match = strstr(searched, search_for);
-
- /*
-* Stop looking if no new match is found or looking past the
-* searched_start pointer
-*/
- if (match == NULL || (searched_start != NULL
- match + strlen(search_for) searched_start))
+ const char *match = strstr(cur_searched, search_for);
+ const char *prevch;
+ const char *nextch;
+
+ /* Stop looking if no new match is found */
+ if (match == NULL)
break;
- result = match;
- searched = match + 1;
+ prevch = match - 1;
+ nextch = match + strlen(search_for);
+
+ /* Skip spaces */
+ while (*prevch == ' ')
+ prevch--;
+ while (*nextch == ' ')
+ nextch++;
+
+ /* Start looking past the current match so last is found */
+ cur_searched = match + 1;
+
+ /* Check for an exact match */
+ if (match != searched
+ *prevch != ENV_ATTR_LIST_DELIM)
+ continue;
+ if (*nextch != ENV_ATTR_SEP
+ *nextch != ENV_ATTR_LIST_DELIM
+ *nextch != '\0')
+ continue;
+
+ *result = match;
+ result_size = strlen(search_for);
}
- return result;
+ return result_size;
}
/*
@@ -145,6 +167,7 @@ static char *reverse_strstr(const char *searched, const
char *search_for,
int env_attr_lookup(const char *attr_list, const char *name, char
*attributes)
{
const char *entry = NULL;
+ int entry_len;
if (!attributes)
/* bad parameter */
@@ -153,32 +176,12 @@ int env_attr_lookup(const char *attr_list, const char
*name, char *attributes)
/* list not found */
return -EINVAL;
- entry = reverse_strstr(attr_list, name, NULL);
- while (entry != NULL) {
- const char *prevch = entry - 1;
- const char *nextch = entry + strlen(name);
-
- /* Skip spaces */
- while (*prevch == ' ')
- prevch--;
- while (*nextch == ' ')
- nextch++;
-
- /* check for an exact match */
- if ((entry == attr_list ||
-*prevch == ENV_ATTR_LIST_DELIM)
- (*nextch == ENV_ATTR_SEP ||
-*nextch == ENV_ATTR_LIST_DELIM ||
-*nextch == '\0'))
- break;
-
- entry = reverse_strstr(attr_list, name, entry);
- }
+ entry_len = reverse_name_search(attr_list, name, entry);
if (entry != NULL) {
int len;
/* skip the name */
- entry += strlen(name);
+ entry += entry_len;
/* skip spaces */
while (*entry == ' ')
entry++;
--
1.7.11.5
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 05/11] env: Simplify the reverse_strstr() interface
The logic to find the whole matching name was split needlessly between
the reverse_strstr function and its caller. Fully contain it to make the
interface for calling it more consistent.
Signed-off-by: Joe Hershberger joe.hershber...@ni.com
---
common/env_attr.c | 79 +--
1 file changed, 41 insertions(+), 38 deletions(-)
diff --git a/common/env_attr.c b/common/env_attr.c
index e791f44..d266142 100644
--- a/common/env_attr.c
+++ b/common/env_attr.c
@@ -109,33 +109,55 @@ int env_attr_walk(const char *attr_list,
}
/*
- * Search for the last matching string in another string with the option to
- * start looking at a certain point (i.e. ignore anything beyond that point).
+ * Search for the last exactly matching name in an attribute list
*/
-static char *reverse_strstr(const char *searched, const char *search_for,
- const char *searched_start)
+static int reverse_name_search(const char *searched, const char *search_for,
+ const char **result)
{
- char *result = NULL;
+ int result_size = 0;
+ const char *cur_searched = searched;
+
+ if (result)
+ *result = NULL;
if (*search_for == '\0')
return (char *)searched;
for (;;) {
- char *match = strstr(searched, search_for);
-
- /*
-* Stop looking if no new match is found or looking past the
-* searched_start pointer
-*/
- if (match == NULL || (searched_start != NULL
- match + strlen(search_for) searched_start))
+ const char *match = strstr(cur_searched, search_for);
+ const char *prevch;
+ const char *nextch;
+
+ /* Stop looking if no new match is found */
+ if (match == NULL)
break;
- result = match;
- searched = match + 1;
+ prevch = match - 1;
+ nextch = match + strlen(search_for);
+
+ /* Skip spaces */
+ while (*prevch == ' ')
+ prevch--;
+ while (*nextch == ' ')
+ nextch++;
+
+ /* Start looking past the current match so last is found */
+ cur_searched = match + 1;
+
+ /* Check for an exact match */
+ if (match != searched
+ *prevch != ENV_ATTR_LIST_DELIM)
+ continue;
+ if (*nextch != ENV_ATTR_SEP
+ *nextch != ENV_ATTR_LIST_DELIM
+ *nextch != '\0')
+ continue;
+
+ *result = match;
+ result_size = strlen(search_for);
}
- return result;
+ return result_size;
}
/*
@@ -145,6 +167,7 @@ static char *reverse_strstr(const char *searched, const
char *search_for,
int env_attr_lookup(const char *attr_list, const char *name, char *attributes)
{
const char *entry = NULL;
+ int entry_len;
if (!attributes)
/* bad parameter */
@@ -153,32 +176,12 @@ int env_attr_lookup(const char *attr_list, const char
*name, char *attributes)
/* list not found */
return -EINVAL;
- entry = reverse_strstr(attr_list, name, NULL);
- while (entry != NULL) {
- const char *prevch = entry - 1;
- const char *nextch = entry + strlen(name);
-
- /* Skip spaces */
- while (*prevch == ' ')
- prevch--;
- while (*nextch == ' ')
- nextch++;
-
- /* check for an exact match */
- if ((entry == attr_list ||
-*prevch == ENV_ATTR_LIST_DELIM)
- (*nextch == ENV_ATTR_SEP ||
-*nextch == ENV_ATTR_LIST_DELIM ||
-*nextch == '\0'))
- break;
-
- entry = reverse_strstr(attr_list, name, entry);
- }
+ entry_len = reverse_name_search(attr_list, name, entry);
if (entry != NULL) {
int len;
/* skip the name */
- entry += strlen(name);
+ entry += entry_len;
/* skip spaces */
while (*entry == ' ')
entry++;
--
1.7.11.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
