From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id E28A61F5AE for ; Tue, 16 Jun 2020 12:47:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728672AbgFPMrl (ORCPT ); Tue, 16 Jun 2020 08:47:41 -0400 Received: from cloud.peff.net ([104.130.231.41]:33040 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726692AbgFPMrk (ORCPT ); Tue, 16 Jun 2020 08:47:40 -0400 Received: (qmail 21574 invoked by uid 109); 16 Jun 2020 12:47:40 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 16 Jun 2020 12:47:40 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 12820 invoked by uid 111); 16 Jun 2020 12:47:39 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Tue, 16 Jun 2020 08:47:39 -0400 Authentication-Results: peff.net; auth=none Date: Tue, 16 Jun 2020 08:47:39 -0400 From: Jeff King To: Eric Sunshine Cc: Don Goodman-Wilson via GitGitGadget , Git List , Derrick Stolee , "brian m. carlson" , Johannes Schindelin , Don Goodman-Wilson Subject: Re: [PATCH 1/9] init: allow overriding the default branch name for new repositories Message-ID: <20200616124739.GD666057@coredump.intra.peff.net> References: <90912e32da1192cfc3b39a18cb606caa46e85b1c.1591823971.git.gitgitgadget@gmail.com> <20200616124502.GC666057@coredump.intra.peff.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200616124502.GC666057@coredump.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Tue, Jun 16, 2020 at 08:45:02AM -0400, Jeff King wrote: > On Wed, Jun 10, 2020 at 08:16:38PM -0400, Eric Sunshine wrote: > > > > +/* > > > + * 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()); > } Actually, one small hiccup is that the config option specifies the branch name, not the ref name. So you really would have to prepare a static-owned copy of it to turn "foo" into "refs/heads/foo" to get the refname. On the other hand, that would also be a good time to run check_ref_format(). In the patch as-is, the "short" return does not check that the branch is a valid name. -Peff