Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to do a code review.
Please visit
https://gerrit.openvpn.net/c/openvpn/+/1987?usp=email
to review the following change.
Change subject: buffer: Clean up rm_trailing_chars
......................................................................
buffer: Clean up rm_trailing_chars
This is very weird code. We re-run strlen for
every char that we delete, even though we actually
know which char to test next. Avoid this.
Also add unit tests to make sure I did not break it.
Change-Id: I47e0741c75832a8f3fe161769feb27ddbfba3cdd
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/buffer.c
M tests/unit_tests/openvpn/test_buffer.c
2 files changed, 48 insertions(+), 12 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/87/1987/1
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 8f558b1..9b87f42 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -592,21 +592,18 @@
void
rm_trailing_chars(char *str, const char *what_to_delete)
{
- bool modified;
- do
+ size_t len = strlen(str);
+ while (len > 0)
{
- const size_t len = strlen(str);
- modified = false;
- if (len > 0)
+ if (strchr(what_to_delete, str[len - 1]) != NULL)
{
- char *cp = str + (len - 1);
- if (strchr(what_to_delete, *cp) != NULL)
- {
- *cp = '\0';
- modified = true;
- }
+ str[--len] = '\0';
}
- } while (modified);
+ else
+ {
+ return;
+ }
+ }
}
/*
diff --git a/tests/unit_tests/openvpn/test_buffer.c
b/tests/unit_tests/openvpn/test_buffer.c
index 2945e8a..bf486e0 100644
--- a/tests/unit_tests/openvpn/test_buffer.c
+++ b/tests/unit_tests/openvpn/test_buffer.c
@@ -435,6 +435,44 @@
}
void
+test_string_chomp(void **state)
+{
+ char string[10];
+
+ strcpy(string, "Foo\n");
+ chomp(string);
+ assert_string_equal(string, "Foo");
+
+ strcpy(string, "Foo\r\n\n\r");
+ chomp(string);
+ assert_string_equal(string, "Foo");
+
+ strcpy(string, "Foo");
+ chomp(string);
+ assert_string_equal(string, "Foo");
+
+ strcpy(string, "F\ro\no");
+ chomp(string);
+ assert_string_equal(string, "F\ro\no");
+
+ string[2] = '\0';
+ chomp(string);
+ assert_string_equal(string, "F");
+
+ strcpy(string, "");
+ chomp(string);
+ assert_string_equal(string, "");
+
+ strcpy(string, "Foo");
+ rm_trailing_chars(string, "o");
+ assert_string_equal(string, "F");
+
+ strcpy(string, "Foo");
+ rm_trailing_chars(string, "oF");
+ assert_string_equal(string, "");
+}
+
+void
test_buffer_chomp(void **state)
{
struct gc_arena gc = gc_new();
@@ -616,6 +654,7 @@
cmocka_unit_test(test_character_string_mod_buf),
cmocka_unit_test(test_snprintf),
cmocka_unit_test(test_checked_snprintf),
+ cmocka_unit_test(test_string_chomp),
cmocka_unit_test(test_buffer_chomp),
cmocka_unit_test(test_buffer_null_terminate),
cmocka_unit_test(test_buffer_null_predicates),
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/1987?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I47e0741c75832a8f3fe161769feb27ddbfba3cdd
Gerrit-Change-Number: 1987
Gerrit-PatchSet: 1
Gerrit-Owner: Frank Lichtenheld <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
