Re: [PATCH 1/9] init: allow overriding the default branch name for new repositories

From: Jeff King <peff@peff.net>
Date: Jun 16, 2020, 12:45 UTC
Message-ID: <20200616124502.GC666057@coredump.intra.peff.net>
In-Reply-To: <CAPig+cSnEvVB5vsffFXidG1-XNxDX10u2XhD9NqV3pwh8zyxxw@mail.gmail.com>

On Wed, Jun 10, 2020 at 08:16:38PM -0400, Eric Sunshine wrote:

Show 13 quoted lines

> > +/*
> > + * Retrieves the name of the default branch. If `short_name` is non-zero, the
> > + * branch name will be prefixed with "refs/heads/".
> > + */
> > +char *git_default_branch_name(int short_name);
> 
> Overall, the internal logic regarding duplicating/freeing strings
> would probably be easier to grok if there were two separate functions:
> 
>     char *git_default_branch_name(void);
>     char *git_default_ref_name(void);
> 
> but that's subjective.

Having seen one of the callers, might it be worth avoiding handing off ownership of the string entirely?

I.e., this comes from a string that's already owned for the lifetime of the process (either the environment, or a string stored by the config machinery). Could we just pass that back (or if we want to be more careful about getenv() lifetimes, we can copy it into a static owned by this function)?

Then all of the callers can stop dealing with the extra free(), and you can do:

  const char *git_default_branch_name(void)
  {
	return skip_prefix("refs/heads/", git_default_ref_name());
  }

-Peff