Re: [RFC] [GSoC]: STRBUF_INIT_CONST: initialize `strbuf` to constant string

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 29, 2026, 05:23 UTC
Message-ID: <CAPig+cTmvu+tmuvb-h+VsA8NL5xJgf6XPZGnERVqh1cp40hV_w@mail.gmail.com>
In-Reply-To: <CAFRsFoWRRnbrJdp_HVuoW-AEMqz_XjoP5yFAFP73VVN9nhdp2w@mail.gmail.com>

On Sat, Mar 28, 2026 at 5:41 PM Mateo Patino <mateopatinodev@gmail.com> wrote:

Show 19 quoted lines

> > [...] So, this suggests a
> > dedicated, simple structure and a few related functions and a macro or
> > two. For instance, something like this:
> >
> >   struct str {
> >     char *s;
> >     int free_me;
> >   };
>
> Thanks for explaining the original problem in such detail, I see I really
> hadn't completely understood what the original problem "X" was.
>
> To clarify, you are imagining this `struct str` more as a "smart pointer"
> than a full string abstraction, correct? I was going to propose including a
> `size_t len` member for this struct, but after some thought, I feel like that
> would somewhat transform `struct str` into a string abstraction, which `strbuf`
> already is. The way you're imagining `struct str` could be used around in the
> Git codebase is as a wrapper whose only purpose is to inform clients of
> a string's ownership, correct?

You're correct that I'm not proposing a full string abstraction; however, I wouldn't exactly call it a smart-pointer or say that it "informs" clients of a string's ownership. It's just a tool which makes it simple for clients to reassign the string without having to worry about the ownership.

Whether or not it would be generally helpful throughout the Git codebase remains to be seen.

Show 22 quoted lines

> >   /* initialize `str` from a literal string (i.e. "foo") */
> >   #define STR_INIT(X) { .s = (char *)(X), .free_me = 0 }
> >
> >   void str_release(str *x) {
> >     if (x.free_me)
> >       FREE_AND_NULL(x.s);
> >     x.free_me = 0;
> >   }
> >
> >   /* take ownership of a heap-allocated string */
> >   void str_take(str *x, char * s) {
> >     str_release(x);
> >     x.s = s;
> >     x.free_me = 1;
> >   }
> >
> >   /* assign a string literal (i.e. "foo") */
> >   void str_assign(str *x, const char *s) {
> >     str_release(x);
> >     x.s = (char *)s;
> >     x.free_me = 0;
> >   }

By the way, the above example using the "free_me" member was just for illustration purposes since "free_me" makes the ownership concerns obvious. However, in practice, a better approach would be to employ the "to_free" idiom which is used elsewhere in the Git codebase since it avoids all the ugly casts. Something like this:

  struct str {
    const char *s;
    char *to_free; /* private */
  };

  /* initialize `str` from a literal string (i.e. "foo") */
  #define STR_INIT(X) { .s = (X), .to_free = NULL }

  void str_release(struct str *x) {
    x.s = NULL;
    FREE_AND_NULL(x.to_free);
  }

  /* take ownership of a heap-allocated string */
  void str_take(struct str *x, char *s) {
    str_release(x);
    x.s = s;
    x.to_free = s;
  }

  /* assign a string literal (i.e. "foo") */
  void str_assign(struct str *x, const char *s) {
    str_release(x);
    x.s = s;
  }

Show 11 quoted lines

> > Clients which need the value simply access the `.s` member directly.
> > And there is no need to have any functions to morph the string in any
> > way. If a client needs that functionality, it is easy enough to create
> > and populate a proper `strbuf` from the `.s` member.
>
> So if we were to make this into a patch, would we implement this as a local
> helper in config.c, where the original problem started? I imagine this small
> ownership interface could likely be used in multiple places around the codebase,
> so my first instinct would be to not restrict it to config.c. Would it be
> too premature to give this `struct str` its own module? If so, then how would an
> idea of this sort be first presented to the community as a patch?

My gut feeling is that it would make sense first to introduce such a utility locally in `config.c` where it is needed. If it becomes apparent that it has value outside of `config.c`, then it could be extracted into a reusable component. However, others may feel differently, and even I don't feel strongly about it.

One reason I hesitate to suggest that this would be generally useful is that the existing "to_free" idiom employed in Git is already about as simple as it gets, and I don't think the proposed "str" utility would necessarily make it any simpler or improve code quality. For instance, a typical use of "to_free" might be something like this:

  const char *name = "default";
  char *to_free = NULL;
  ...do stuff...
  if (some_condition)
    name = to_free = xstrdup(some_str_var);
  ...do stuff...
  free(to_free);

Changing this to take advantage of the proposed "str" might result in:

  struct str name = STR_INIT("default");
  ...do stuff...
  if (some_condition)
    str_take(&name, xstrdup(some_str_var));
  ...do stuff...
  str_release(&name);

which is only one line shorter, and not necessarily any clearer or less noisy. So, there isn't a strong reason (outside of `config.c`) to convert such code to use "str", and any such conversion just for the sake of conversion would probably be unwanted churn.