ref-filter: factor out refname component counting

The "lstrip" and "rstrip" options to the %(refname) placeholder both
accept a negative length, which asks us to keep that many path
components (rather than stripping that many).

The code to count components and convert the negative value to a
positive was copied from lstrip to rstrip in 1a34728e6b (ref-filter: add
an 'rstrip=<N>' option to atoms which deal with refnames, 2017-01-10).

Let's factor it out into a separate function. This reduces duplication
and also makes the lstrip/rstrip functions much easier to follow, since
the bulk of their code is now the actual stripping.

Note that the computed "remaining" value is currently stored as a
"long", so in theory that's what our function should return. But this is
purely historical. When the variable was added in 0571979bd6 (tag: do
not show ambiguous tag names as "tags/foo", 2016-01-25), we parsed the
value from strtol(), and thus used a long. But these days we take "len"
as an int, and also use an int to count up components. So let's just
consistently use int here. This value could only overflow in a
pathological case (e.g., 4GB worth of "a/a/...") and even then will not
result in out-of-bounds memory access (we keep stripping until we run
out of string to parse).

The minimal Myers diff here is a little hard to read; with --patience
the code movement is shown much more clearly.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/ref-filter.c b/ref-filter.c
index c318f9ca0ec8dd6f4d877ba2f89390e3272ec52e..ff14ac53de2ed2e2d126f5644db42e510bef213f 100644 (file)
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2173,12 +2173,8 @@ static inline char *copy_advance(char *dst, const char *src)
        return dst;
 }
 
-static const char *lstrip_ref_components(const char *refname, int len)
+static int normalize_component_count(const char *refname, int len)
 {
-       long remaining = len;
-       const char *start = xstrdup(refname);
-       const char *to_free = start;
-
        if (len < 0) {
                int i;
                const char *p = refname;
@@ -2192,8 +2188,16 @@ static const char *lstrip_ref_components(const char *refname, int len)
                 * because we count the number of '/', but the number
                 * of components is one more than the no of '/').
                 */
-               remaining = i + len + 1;
+               len = i + len + 1;
        }
+       return len;
+}
+
+static const char *lstrip_ref_components(const char *refname, int len)
+{
+       int remaining = normalize_component_count(refname, len);
+       const char *start = xstrdup(refname);
+       const char *to_free = start;
 
        while (remaining > 0) {
                switch (*start++) {
@@ -2213,26 +2217,10 @@ static const char *lstrip_ref_components(const char *refname, int len)
 
 static const char *rstrip_ref_components(const char *refname, int len)
 {
-       long remaining = len;
+       int remaining = normalize_component_count(refname, len);
        const char *start = xstrdup(refname);
        const char *to_free = start;
 
-       if (len < 0) {
-               int i;
-               const char *p = refname;
-
-               /* Find total no of '/' separated path-components */
-               for (i = 0; p[i]; p[i] == '/' ? i++ : *p++)
-                       ;
-               /*
-                * The number of components we need to strip is now
-                * the total minus the components to be left (Plus one
-                * because we count the number of '/', but the number
-                * of components is one more than the no of '/').
-                */
-               remaining = i + len + 1;
-       }
-
        while (remaining-- > 0) {
                char *p = strrchr(start, '/');
                if (!p) {