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>

diff --git a/builtin/clean.c b/builtin/clean.c
index df53def63f29e295e09261b9bd56960cf27582b1..d7acb94a9594563b1473ecac29bd5de9a5e8a55a 100644 (file)
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -159,8 +159,7 @@ static int is_git_repository(struct strbuf *path)
        int gitfile_error;
        size_t orig_path_len = path->len;
        assert(orig_path_len != 0);
-       if (path->buf[orig_path_len - 1] != '/')
-               strbuf_addch(path, '/');
+       strbuf_complete(path, '/');
        strbuf_addstr(path, ".git");
        if (read_gitfile_gently(path->buf, &gitfile_error) || is_git_directory(path->buf))
                ret = 1;
@@ -206,8 +205,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
                return res;
        }
 
-       if (path->buf[original_len - 1] != '/')
-               strbuf_addch(path, '/');
+       strbuf_complete(path, '/');
 
        len = path->len;
        while ((e = readdir(dir)) != NULL) {

diff --git a/builtin/log.c b/builtin/log.c
index a491d3dea0e412624e7212060658e22a421cdfbf..dda671d975255494a6987e82e2ccf23f7f69f7ee 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -796,8 +796,7 @@ static int reopen_stdout(struct commit *commit, const char *subject,
                if (filename.len >=
                    PATH_MAX - FORMAT_PATCH_NAME_MAX - suffix_len)
                        return error(_("name of output directory is too long"));
-               if (filename.buf[filename.len - 1] != '/')
-                       strbuf_addch(&filename, '/');
+               strbuf_complete(&filename, '/');
        }
 
        if (rev->numbered_files)

diff --git a/diff-no-index.c b/diff-no-index.c
index 0320605a84178ffb2ab9384b8ee9fbac7df16a73..8e0fd270b5be4d9ab8616ab57202862dd2fc6eda 100644 (file)
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -136,15 +136,13 @@ static int queue_diff(struct diff_options *o,
 
                if (name1) {
                        strbuf_addstr(&buffer1, name1);
-                       if (buffer1.len && buffer1.buf[buffer1.len - 1] != '/')
-                               strbuf_addch(&buffer1, '/');
+                       strbuf_complete(&buffer1, '/');
                        len1 = buffer1.len;
                }
 
                if (name2) {
                        strbuf_addstr(&buffer2, name2);
-                       if (buffer2.len && buffer2.buf[buffer2.len - 1] != '/')
-                               strbuf_addch(&buffer2, '/');
+                       strbuf_complete(&buffer2, '/');
                        len2 = buffer2.len;
                }
 

diff --git a/dir.c b/dir.c
index 7b25634832716ade46686d118184c4d43d8c11e7..79fdad84251368db1e851814f2a6f9064076e0d6 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -1519,8 +1519,7 @@ static enum path_treatment treat_path_fast(struct dir_struct *dir,
        }
        strbuf_addstr(path, cdir->ucd->name);
        /* treat_one_path() does this before it calls treat_directory() */
-       if (path->buf[path->len - 1] != '/')
-               strbuf_addch(path, '/');
+       strbuf_complete(path, '/');
        if (cdir->ucd->check_only)
                /*
                 * 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)
                else
                        return -1;
        }
-       if (path->buf[original_len - 1] != '/')
-               strbuf_addch(path, '/');
+       strbuf_complete(path, '/');
 
        len = path->len;
        while ((e = readdir(dir)) != NULL) {

diff --git a/path.c b/path.c
index c597473e62265d1b59510c15a90617256a9897c3..c105a9e083307395871ccf3b615c92ad0754fdd7 100644 (file)
--- a/path.c
+++ b/path.c
@@ -240,8 +240,7 @@ static void do_submodule_path(struct strbuf *buf, const char *path,
        const char *git_dir;
 
        strbuf_addstr(buf, path);
-       if (buf->len && buf->buf[buf->len - 1] != '/')
-               strbuf_addch(buf, '/');
+       strbuf_complete(buf, '/');
        strbuf_addstr(buf, ".git");
 
        git_dir = read_gitfile(buf->buf);

diff --git a/refs.c b/refs.c
index 9937a40484503b068f52c940e8b62325b9edfae0..b2a922945d2a5e6adc96da2ba96ba1be90c03cce 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -2193,8 +2193,7 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 
        if (!has_glob_specials(pattern)) {
                /* Append implied '/' '*' if not present. */
-               if (real_pattern.buf[real_pattern.len - 1] != '/')
-                       strbuf_addch(&real_pattern, '/');
+               strbuf_complete(&real_pattern, '/');
                /* No need to check for '*', there is none. */
                strbuf_addch(&real_pattern, '*');
        }

diff --git a/url.c b/url.c
index 7ca2a69e1091fc57cb292052015c920499fe3379..2d89ad190cfe1c57cd20194661329551ff229993 100644 (file)
--- a/url.c
+++ b/url.c
@@ -120,8 +120,7 @@ char *url_decode_parameter_value(const char **query)
 void end_url_with_slash(struct strbuf *buf, const char *url)
 {
        strbuf_addstr(buf, url);
-       if (buf->len && buf->buf[buf->len - 1] != '/')
-               strbuf_addch(buf, '/');
+       strbuf_complete(buf, '/');
 }
 
 void str_end_url_with_slash(const char *url, char **dest) {