From: Frank Lichtenheld <[email protected]> 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]> Acked-by: Razvan Cojocaru <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1987 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1987 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Razvan Cojocaru <[email protected]> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 8f558b1..a90ddae 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -592,21 +592,11 @@ void rm_trailing_chars(char *str, const char *what_to_delete) { - bool modified; - do + size_t len = strlen(str); + while (len > 0 && strchr(what_to_delete, str[len - 1]) != NULL) { - const size_t len = strlen(str); - modified = false; - if (len > 0) - { - char *cp = str + (len - 1); - if (strchr(what_to_delete, *cp) != NULL) - { - *cp = '\0'; - modified = true; - } - } - } while (modified); + str[--len] = '\0'; + } } /* 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), _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
