@matheusbsbs / matheus-bagliano / commits / 00b6c178c3

use strbuf_complete to conditionally append slash

When working with paths in strbufs, we frequently want to ensure that a directory contains a trailing slash before appending to it. We can shorten this code (and make the intent more obvious) by calling strbuf_complete. Most of these cases are trivially identical conversions, but there are two things to note: - in a few cases we did not check that the strbuf is non-empty (which would lead to an out-of-bounds memory access). These were generally not triggerable in practice, either from earlier assertions, or typically because we would have just fed the strbuf to opendir(), which would choke on an empty path. - in a few cases we indexed the buffer with "original_len" or similar, rather than the current sb->len, and it is not immediately obvious from the diff that they are the same. In all of these cases, I manually verified that the strbuf does not change between the assignment and the strbuf_complete call. This does not convert cases which look like: if (sb->len && !is_dir_sep(sb->buf[sb->len - 1])) strbuf_addch(sb, '/'); as those are obviously semantically different. Some of these cases arguably should be doing that, but that is out of scope for this change, which aims purely for cleanup with no behavior change (and at least it will make such sites easier to find and examine in the future, as we can grep for strbuf_complete). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 24, 2015 at 17:08 UTC 00b6c178c3ab475098f7a0bc63b2df2da508020c

7 files changed +10 -20

builtin/clean.c

+2 -4

@@ -159,8 +159,7 @@ static int is_git_repository(struct strbuf *path)

159 int gitfile_error;

160 size_t orig_path_len = path->len;

161 assert(orig_path_len != 0);

162 - if (path->buf[orig_path_len - 1] != '/')

163 - strbuf_addch(path, '/');

162 + strbuf_complete(path, '/');

163 strbuf_addstr(path, ".git");

164 if (read_gitfile_gently(path->buf, &gitfile_error) || is_git_directory(path->buf))

165 ret = 1;

@@ -206,8 +205,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,

205 return res;

206 }

207

209 - if (path->buf[original_len - 1] != '/')

210 - strbuf_addch(path, '/');

208 + strbuf_complete(path, '/');

209

210 len = path->len;

211 while ((e = readdir(dir)) != NULL) {

builtin/log.c

+1 -2

@@ -796,8 +796,7 @@ static int reopen_stdout(struct commit *commit, const char *subject,

796 if (filename.len >=

797 PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len)

798 return error(_("name of output directory is too long"));

799 - if (filename.buf[filename.len - 1] != '/')

800 - strbuf_addch(&filename, '/');

799 + strbuf_complete(&filename, '/');

800 }

801

802 if (rev->numbered_files)

diff-no-index.c

+2 -4

@@ -136,15 +136,13 @@ static int queue_diff(struct diff_options *o,

136

137 if (name1) {

138 strbuf_addstr(&buffer1, name1);

139 - if (buffer1.len && buffer1.buf[buffer1.len - 1] != '/')

140 - strbuf_addch(&buffer1, '/');

139 + strbuf_complete(&buffer1, '/');

140 len1 = buffer1.len;

141 }

142

143 if (name2) {

144 strbuf_addstr(&buffer2, name2);

146 - if (buffer2.len && buffer2.buf[buffer2.len - 1] != '/')

147 - strbuf_addch(&buffer2, '/');

145 + strbuf_complete(&buffer2, '/');

146 len2 = buffer2.len;

147 }

148

dir.c

+2 -4

@@ -1519,8 +1519,7 @@ static enum path_treatment treat_path_fast(struct dir_struct *dir,

1519 }

1520 strbuf_addstr(path, cdir->ucd->name);

1521 /* treat_one_path() does this before it calls treat_directory() */

1522 - if (path->buf[path->len - 1] != '/')

1523 - strbuf_addch(path, '/');

1522 + strbuf_complete(path, '/');

1523 if (cdir->ucd->check_only)

1524 /*

1525 * check_only is set as a result of treat_directory() getting

@@ -2126,8 +2125,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)

2125 else

2126 return -1;

2127 }

2129 - if (path->buf[original_len - 1] != '/')

2130 - strbuf_addch(path, '/');

2128 + strbuf_complete(path, '/');

2129

2130 len = path->len;

2131 while ((e = readdir(dir)) != NULL) {

path.c

+1 -2

@@ -240,8 +240,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,

240 const char *git_dir;

241

242 strbuf_addstr(buf, path);

243 - if (buf->len && buf->buf[buf->len - 1] != '/')

244 - strbuf_addch(buf, '/');

243 + strbuf_complete(buf, '/');

244 strbuf_addstr(buf, ".git");

245

246 git_dir = read_gitfile(buf->buf);

refs.c

+1 -2

@@ -2193,8 +2193,7 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,

2193

2194 if (!has_glob_specials(pattern)) {

2195 /* Append implied '/' '*' if not present. */

2196 - if (real_pattern.buf[real_pattern.len - 1] != '/')

2197 - strbuf_addch(&real_pattern, '/');

2196 + strbuf_complete(&real_pattern, '/');

2197 /* No need to check for '*', there is none. */

2198 strbuf_addch(&real_pattern, '*');

2199 }

url.c

+1 -2

@@ -120,8 +120,7 @@ char *url_decode_parameter_value(const char **query)

120 void end_url_with_slash(struct strbuf *buf, const char *url)

121 {

122 strbuf_addstr(buf, url);

123 - if (buf->len && buf->buf[buf->len - 1] != '/')

124 - strbuf_addch(buf, '/');

123 + strbuf_complete(buf, '/');

124 }

125

126 void str_end_url_with_slash(const char *url, char **dest) {