Merge pull request #504 from arrbee/git-buf-fast-join Optimized implementation of git_buf_join
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217
diff --git a/src/buffer.c b/src/buffer.c
index 3dff813..b90dd29 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -282,58 +282,24 @@ void git_buf_join(
const char *str_a,
const char *str_b)
{
- size_t add_size = 0;
- size_t sep_a = 0;
- size_t strlen_a = 0;
- size_t sep_b = 0;
- size_t strlen_b = 0;
- char *ptr;
-
- /* calculate string lengths and need for added separators */
- if (str_a) {
- while (*str_a == separator) { sep_a = 1; str_a++; }
- strlen_a = strlen(str_a);
+ size_t strlen_a = strlen(str_a);
+ size_t strlen_b = strlen(str_b);
+ int need_sep = 0;
+
+ /* figure out if we need to insert a separator */
+ if (separator && strlen_a) {
+ while (*str_b == separator) { str_b++; strlen_b--; }
+ if (str_a[strlen_a - 1] != separator)
+ need_sep = 1;
}
- if (str_b) {
- while (*str_b == separator) { sep_b = 1; str_b++; }
- strlen_b = strlen(str_b);
- }
- if (buf->size > 0) {
- if (buf->ptr[buf->size - 1] == separator) {
- sep_a = 0;
- if (!strlen_a) sep_b = 0;
- } else if (!strlen_a)
- sep_b = (str_b != NULL);
- }
- if (strlen_a > 0) {
- if (str_a[strlen_a - 1] == separator)
- sep_b = 0;
- else if (str_b)
- sep_b = 1;
- }
-
- add_size = sep_a + strlen_a + sep_b + strlen_b;
- if (!add_size) return;
+ ENSURE_SIZE(buf, strlen_a + strlen_b + need_sep + 1);
- ENSURE_SIZE(buf, buf->size + add_size + 1);
+ memmove(buf->ptr, str_a, strlen_a);
+ if (need_sep)
+ buf->ptr[strlen_a] = separator;
+ memmove(buf->ptr + strlen_a + need_sep, str_b, strlen_b);
- /* concatenate strings */
- ptr = buf->ptr + buf->size;
- if (sep_a)
- *ptr++ = separator;
- if (strlen_a) {
- memmove(ptr, str_a, strlen_a);
- ptr += strlen_a;
- }
- if (sep_b)
- *ptr++ = separator;
- if (strlen_b) {
- memmove(ptr, str_b, strlen_b);
- ptr += strlen_b;
- }
-
- /* set size based on num characters actually written */
- buf->size = ptr - buf->ptr;
+ buf->size = strlen_a + strlen_b + need_sep;
buf->ptr[buf->size] = '\0';
}
diff --git a/tests-clay/clay.h b/tests-clay/clay.h
index e75ef85..b3aae46 100644
--- a/tests-clay/clay.h
+++ b/tests-clay/clay.h
@@ -92,6 +92,7 @@ extern void test_core_buffer__5(void);
extern void test_core_buffer__6(void);
extern void test_core_buffer__7(void);
extern void test_core_buffer__8(void);
+extern void test_core_buffer__9(void);
extern void test_core_dirent__dont_traverse_dot(void);
extern void test_core_dirent__dont_traverse_empty_folders(void);
extern void test_core_dirent__traverse_slash_terminated_folder(void);
diff --git a/tests-clay/clay_libgit2.h b/tests-clay/clay_libgit2.h
index 2eedc61..6b14b7d 100644
--- a/tests-clay/clay_libgit2.h
+++ b/tests-clay/clay_libgit2.h
@@ -29,15 +29,16 @@
* Wrapper for string comparison that knows about nulls.
*/
#define cl_assert_strequal(a,b) \
- cl_assert_strequal_internal(a,b,__FILE__,__LINE__)
+ cl_assert_strequal_internal(a,b,__FILE__,__LINE__,"string mismatch: " #a " != " #b)
-GIT_INLINE(void) cl_assert_strequal_internal(const char *a, const char *b, const char *file, int line)
+GIT_INLINE(void) cl_assert_strequal_internal(
+ const char *a, const char *b, const char *file, int line, const char *err)
{
int match = (a == NULL || b == NULL) ? (a == b) : (strcmp(a, b) == 0);
if (!match) {
char buf[4096];
snprintf(buf, 4096, "'%s' != '%s'", a, b);
- clay__assert(0, file, line, buf, "Strings do not match", 1);
+ clay__assert(0, file, line, buf, err, 1);
}
}
diff --git a/tests-clay/clay_main.c b/tests-clay/clay_main.c
index 65c56de..409ce76 100644
--- a/tests-clay/clay_main.c
+++ b/tests-clay/clay_main.c
@@ -145,7 +145,8 @@ static const struct clay_func _clay_cb_core_buffer[] = {
{"5", &test_core_buffer__5},
{"6", &test_core_buffer__6},
{"7", &test_core_buffer__7},
- {"8", &test_core_buffer__8}
+ {"8", &test_core_buffer__8},
+ {"9", &test_core_buffer__9}
};
static const struct clay_func _clay_cb_core_dirent[] = {
{"dont_traverse_dot", &test_core_dirent__dont_traverse_dot},
@@ -326,7 +327,7 @@ static const struct clay_suite _clay_suites[] = {
"core::buffer",
{NULL, NULL},
{NULL, NULL},
- _clay_cb_core_buffer, 9
+ _clay_cb_core_buffer, 10
},
{
"core::dirent",
@@ -493,7 +494,7 @@ static const struct clay_suite _clay_suites[] = {
};
static size_t _clay_suite_count = 34;
-static size_t _clay_callback_count = 112;
+static size_t _clay_callback_count = 113;
/* Core test functions */
static void
diff --git a/tests-clay/core/buffer.c b/tests-clay/core/buffer.c
index acde8c0..7d25b81 100644
--- a/tests-clay/core/buffer.c
+++ b/tests-clay/core/buffer.c
@@ -374,20 +374,10 @@ check_joinbuf_2(
char sep = '/';
git_buf buf = GIT_BUF_INIT;
- /* first validate join from nothing */
git_buf_join(&buf, sep, a, b);
cl_assert(git_buf_oom(&buf) == 0);
cl_assert_strequal(expected, git_buf_cstr(&buf));
git_buf_free(&buf);
-
- /* next validate join-append */
- git_buf_sets(&buf, a);
- cl_assert(git_buf_oom(&buf) == 0);
- git_buf_join(&buf, sep, NULL, b);
- cl_assert(git_buf_oom(&buf) == 0);
- cl_assert_strequal(expected, git_buf_cstr(&buf));
-
- git_buf_free(&buf);
}
static void
@@ -490,3 +480,45 @@ void test_core_buffer__8(void)
check_joinbuf_n_4(";abcd;", ";efgh;", ";ijkl;", ";mnop;", ";abcd;efgh;ijkl;mnop;");
}
+void test_core_buffer__9(void)
+{
+ git_buf buf = GIT_BUF_INIT;
+
+ /* just some exhaustive tests of various separator placement */
+ char *a[] = { "", "-", "a-", "-a", "-a-" };
+ char *b[] = { "", "-", "b-", "-b", "-b-" };
+ char sep[] = { 0, '-', '/' };
+ char *expect_null[] = { "", "-", "a-", "-a", "-a-",
+ "-", "--", "a--", "-a-", "-a--",
+ "b-", "-b-", "a-b-", "-ab-", "-a-b-",
+ "-b", "--b", "a--b", "-a-b", "-a--b",
+ "-b-", "--b-", "a--b-", "-a-b-", "-a--b-" };
+ char *expect_dash[] = { "", "-", "a-", "-a-", "-a-",
+ "-", "-", "a-", "-a-", "-a-",
+ "b-", "-b-", "a-b-", "-a-b-", "-a-b-",
+ "-b", "-b", "a-b", "-a-b", "-a-b",
+ "-b-", "-b-", "a-b-", "-a-b-", "-a-b-" };
+ char *expect_slas[] = { "", "-/", "a-/", "-a/", "-a-/",
+ "-", "-/-", "a-/-", "-a/-", "-a-/-",
+ "b-", "-/b-", "a-/b-", "-a/b-", "-a-/b-",
+ "-b", "-/-b", "a-/-b", "-a/-b", "-a-/-b",
+ "-b-", "-/-b-", "a-/-b-", "-a/-b-", "-a-/-b-" };
+ char **expect_values[] = { expect_null, expect_dash, expect_slas };
+ char separator, **expect;
+ unsigned int s, i, j;
+
+ for (s = 0; s < sizeof(sep) / sizeof(char); ++s) {
+ separator = sep[s];
+ expect = expect_values[s];
+
+ for (j = 0; j < sizeof(b) / sizeof(char*); ++j) {
+ for (i = 0; i < sizeof(a) / sizeof(char*); ++i) {
+ git_buf_join(&buf, separator, a[i], b[j]);
+ cl_assert_strequal(*expect, buf.ptr);
+ expect++;
+ }
+ }
+ }
+
+ git_buf_free(&buf);
+}