* [PATCH] util: Fix skipped characters in strsplit functions
@ 2016-01-07 21:43 Mat Martineau
2016-01-07 23:19 ` Denis Kenzior
0 siblings, 1 reply; 2+ messages in thread
From: Mat Martineau @ 2016-01-07 21:43 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 3507 bytes --]
l_strsplit and l_strsplit_sep were skipping an extra character after
each delimiter because len was incremented after being set to 0 but
before the next iteration of the loop began. This caused problems with
trailing and consecutive delimiters. Added strings with varying
delimiters to tests and made sure len is set back to 0.
---
ell/util.c | 36 ++++++++++++++++++++++--------------
unit/test-string.c | 32 +++++++++++++++++++++++++++++++-
2 files changed, 53 insertions(+), 15 deletions(-)
diff --git a/ell/util.c b/ell/util.c
index 5bd7b3b..d1e7079 100644
--- a/ell/util.c
+++ b/ell/util.c
@@ -293,13 +293,17 @@ LIB_EXPORT char **l_strsplit(const char *str, const char sep)
ret = l_new(char *, len + 1);
- for (i = 0, p = str, len = 0; p[len]; len++) {
- if (p[len] != sep)
- continue;
-
- ret[i++] = l_strndup(p, len);
- p += len + 1;
- len = 0;
+ i = 0;
+ p = str;
+ len = 0;
+ while (p[len]) {
+ if (p[len] != sep) {
+ len++;
+ } else {
+ ret[i++] = l_strndup(p, len);
+ p += len + 1;
+ len = 0;
+ }
}
ret[i++] = l_strndup(p, len);
@@ -347,13 +351,17 @@ LIB_EXPORT char **l_strsplit_set(const char *str, const char *separators)
ret = l_new(char *, len + 1);
- for (i = 0, p = str, len = 0; p[len]; len++) {
- if (sep_table[(unsigned char) p[len]] != true)
- continue;
-
- ret[i++] = l_strndup(p, len);
- p += len + 1;
- len = 0;
+ i = 0;
+ p = str;
+ len = 0;
+ while (p[len]) {
+ if (sep_table[(unsigned char) p[len]] != true) {
+ len++;
+ } else {
+ ret[i++] = l_strndup(p, len);
+ p += len + 1;
+ len = 0;
+ }
}
ret[i++] = l_strndup(p, len);
diff --git a/unit/test-string.c b/unit/test-string.c
index 388e90f..d2f8bd4 100644
--- a/unit/test-string.c
+++ b/unit/test-string.c
@@ -124,6 +124,22 @@ static void test_strsplit(const void *test_data)
assert(!strcmp(strv[2], "bz"));
assert(strv[3] == NULL);
l_strfreev(strv);
+
+ l_strsplit(":bar:bz", ':');
+ assert(strv);
+ assert(!strcmp(strv[0], ""));
+ assert(!strcmp(strv[1], "bar"));
+ assert(!strcmp(strv[2], "bz"));
+ assert(strv[3] == NULL);
+ l_strfreev(strv);
+
+ l_strsplit("Foo:bar:", ':');
+ assert(strv);
+ assert(!strcmp(strv[0], "Foo"));
+ assert(!strcmp(strv[1], "bar"));
+ assert(!strcmp(strv[2], ""));
+ assert(strv[3] == NULL);
+ l_strfreev(strv);
}
static void test_strsplit_set(const void *test_data)
@@ -137,6 +153,20 @@ static void test_strsplit_set(const void *test_data)
assert(!strcmp(strv[3], "Blu"));
assert(strv[4] == NULL);
l_strfreev(strv);
+
+ strv = l_strsplit_set("Foo:bar,Baz Blu,:,Fee:Fie ", ":, ");
+ assert(strv);
+ assert(!strcmp(strv[0], "Foo"));
+ assert(!strcmp(strv[1], "bar"));
+ assert(!strcmp(strv[2], "Baz"));
+ assert(!strcmp(strv[3], "Blu"));
+ assert(!strcmp(strv[4], ""));
+ assert(!strcmp(strv[5], ""));
+ assert(!strcmp(strv[6], "Fee"));
+ assert(!strcmp(strv[7], "Fie"));
+ assert(!strcmp(strv[8], ""));
+ assert(strv[9] == NULL);
+ l_strfreev(strv);
}
static void test_joinv(const void *test_data)
@@ -175,7 +205,7 @@ int main(int argc, char *argv[])
l_test_add("append_fixed test 2", test_fixed, &fixed_test2);
l_test_add("append_fixed test 3", test_fixed, &fixed_test3);
- l_test_add("strplit", test_strsplit, NULL);
+ l_test_add("strsplit", test_strsplit, NULL);
l_test_add("strsplit_set", test_strsplit_set, NULL);
l_test_add("joinv", test_joinv, NULL);
--
2.7.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] util: Fix skipped characters in strsplit functions
2016-01-07 21:43 [PATCH] util: Fix skipped characters in strsplit functions Mat Martineau
@ 2016-01-07 23:19 ` Denis Kenzior
0 siblings, 0 replies; 2+ messages in thread
From: Denis Kenzior @ 2016-01-07 23:19 UTC (permalink / raw)
To: ell
[-- Attachment #1: Type: text/plain, Size: 4473 bytes --]
Hi Mat,
On 01/07/2016 03:43 PM, Mat Martineau wrote:
> l_strsplit and l_strsplit_sep were skipping an extra character after
> each delimiter because len was incremented after being set to 0 but
> before the next iteration of the loop began. This caused problems with
> trailing and consecutive delimiters. Added strings with varying
> delimiters to tests and made sure len is set back to 0.
> ---
Woops :) Indeed it was causing an issue when delimiters were one after
the other. Don't think trailing delimiters were affected. Thanks for
catching this.
> ell/util.c | 36 ++++++++++++++++++++++--------------
> unit/test-string.c | 32 +++++++++++++++++++++++++++++++-
> 2 files changed, 53 insertions(+), 15 deletions(-)
Could you split this patch up into two? One for ell/util.c changes and
one for unit/test-string.c changes. We prefer these to be split
according to top-level directory.
See the newly added HACKING document I just pushed out.
>
> diff --git a/ell/util.c b/ell/util.c
> index 5bd7b3b..d1e7079 100644
> --- a/ell/util.c
> +++ b/ell/util.c
> @@ -293,13 +293,17 @@ LIB_EXPORT char **l_strsplit(const char *str, const char sep)
>
> ret = l_new(char *, len + 1);
>
> - for (i = 0, p = str, len = 0; p[len]; len++) {
> - if (p[len] != sep)
> - continue;
> -
> - ret[i++] = l_strndup(p, len);
> - p += len + 1;
> - len = 0;
> + i = 0;
> + p = str;
> + len = 0;
Empty line here please
> + while (p[len]) {
> + if (p[len] != sep) {
> + len++;
> + } else {
Can we please do:
if (p[len] != sep) {
len += 1;
continue;
}
here?
> + ret[i++] = l_strndup(p, len);
> + p += len + 1;
> + len = 0;
> + }
And leave this chunk unchanged
> }
>
> ret[i++] = l_strndup(p, len);
> @@ -347,13 +351,17 @@ LIB_EXPORT char **l_strsplit_set(const char *str, const char *separators)
>
> ret = l_new(char *, len + 1);
>
> - for (i = 0, p = str, len = 0; p[len]; len++) {
> - if (sep_table[(unsigned char) p[len]] != true)
> - continue;
> -
> - ret[i++] = l_strndup(p, len);
> - p += len + 1;
> - len = 0;
> + i = 0;
> + p = str;
> + len = 0;
> + while (p[len]) {
> + if (sep_table[(unsigned char) p[len]] != true) {
> + len++;
> + } else {
> + ret[i++] = l_strndup(p, len);
> + p += len + 1;
> + len = 0;
> + }
Same as above.
> }
>
> ret[i++] = l_strndup(p, len);
> diff --git a/unit/test-string.c b/unit/test-string.c
> index 388e90f..d2f8bd4 100644
> --- a/unit/test-string.c
> +++ b/unit/test-string.c
> @@ -124,6 +124,22 @@ static void test_strsplit(const void *test_data)
> assert(!strcmp(strv[2], "bz"));
> assert(strv[3] == NULL);
> l_strfreev(strv);
> +
> + l_strsplit(":bar:bz", ':');
> + assert(strv);
> + assert(!strcmp(strv[0], ""));
> + assert(!strcmp(strv[1], "bar"));
> + assert(!strcmp(strv[2], "bz"));
> + assert(strv[3] == NULL);
> + l_strfreev(strv);
> +
> + l_strsplit("Foo:bar:", ':');
> + assert(strv);
> + assert(!strcmp(strv[0], "Foo"));
> + assert(!strcmp(strv[1], "bar"));
> + assert(!strcmp(strv[2], ""));
> + assert(strv[3] == NULL);
> + l_strfreev(strv);
Could you add/modify this test case to include consecutive separators as
well?
> }
>
> static void test_strsplit_set(const void *test_data)
> @@ -137,6 +153,20 @@ static void test_strsplit_set(const void *test_data)
> assert(!strcmp(strv[3], "Blu"));
> assert(strv[4] == NULL);
> l_strfreev(strv);
> +
> + strv = l_strsplit_set("Foo:bar,Baz Blu,:,Fee:Fie ", ":, ");
> + assert(strv);
> + assert(!strcmp(strv[0], "Foo"));
> + assert(!strcmp(strv[1], "bar"));
> + assert(!strcmp(strv[2], "Baz"));
> + assert(!strcmp(strv[3], "Blu"));
> + assert(!strcmp(strv[4], ""));
> + assert(!strcmp(strv[5], ""));
> + assert(!strcmp(strv[6], "Fee"));
> + assert(!strcmp(strv[7], "Fie"));
> + assert(!strcmp(strv[8], ""));
> + assert(strv[9] == NULL);
> + l_strfreev(strv);
> }
>
> static void test_joinv(const void *test_data)
> @@ -175,7 +205,7 @@ int main(int argc, char *argv[])
> l_test_add("append_fixed test 2", test_fixed, &fixed_test2);
> l_test_add("append_fixed test 3", test_fixed, &fixed_test3);
>
> - l_test_add("strplit", test_strsplit, NULL);
> + l_test_add("strsplit", test_strsplit, NULL);
> l_test_add("strsplit_set", test_strsplit_set, NULL);
>
> l_test_add("joinv", test_joinv, NULL);
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-01-07 23:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-07 21:43 [PATCH] util: Fix skipped characters in strsplit functions Mat Martineau
2016-01-07 23:19 ` Denis Kenzior
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.