Re: [PATCH 3/4] Add support for matching full refs in hideRefs

On Sun, 01 Nov 2015 at 21:42:37, Eric Sunshine wrote:

On Sun, Nov 1, 2015 at 2:34 PM, Lukas Fleischer [off-list ref] wrote:

quoted

In addition to matching stripped refs, one can now add hideRefs patterns
that the full (unstripped) ref is matched against. To distinguish
between stripped and full matches, those new patterns must be prefixed
with a circumflex (^).

This commit also removes support for the undocumented and unintended
hideRefs settings "have" (suppressing all "have" lines) and
"capabilities^{}" (suppressing the capabilities line).

Signed-off-by: Lukas Fleischer <redacted>
---

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c

@@ -1198,7 +1200,15 @@ static void reject_updates_to_hidden(struct command *commands)
        struct command *cmd;

        for (cmd = commands; cmd; cmd = cmd->next) {
-               if (cmd->error_string || !ref_is_hidden(cmd->ref_name))

Would it make sense to retain the cmd->error_string check here in
order to avoid doing the extra refname_full construction work below?

    if (cmd->error_string)
        continue;

[...]
This is leaking refname_full each time through the loop since it never
gets free()d. If you restructure the code like this, it might be
easier to avoid leaks:

    for (cmd = ...; ... ; ...) {
        if (cmd->error_string)
            continue;
        strbuf_addf(&refname_full, "%s%s", ...);
        if (ref_is_hidden(...)) {
            if (is_null_sha1(...))
                cmd->error_string = "...";
            else
                cmd->error_string = "...";
        }
        strbuf_release(&refname_full);
    }

As a micro-optimization, you could also pre-populate the strbuf with
get_git_namespace() outside the loop and remember the length. Then,
each time through the loop, just append cmd->ref_name, do your
processing, and, at the bottom of the loop, set the strbuf back to the
remembered length. (And, you still need to free the strbuf after the
loop.)
[...]

Sounds good to me. I changed the code to initialize the strbuf only
once, save the prefix length and reset using strbuf_setlen() in every
loop iteration. Thanks!