Re: [PATCH 0/6] [RFC] Create a 'safe' strbuf API

From: Jeff King <peff@peff.net>
Date: Sep 23, 2026, 19:46 UTC
Message-ID: <20260923194628.GA44327@coredump.intra.peff.net>
In-Reply-To: <9adb1d94-c72b-43f2-aa02-004e3e476eb1@gmail.com>

On Sat, Sep 19, 2026 at 04:23:41PM +0100, Phillip Wood wrote:

Show 12 quoted lines

> > The goal of this short RFC, such as it is, is to get some feedback on
> > whether this is a worthwhile direction to pursue or if I should abandon this
> > idea of having this definition of "safe" for some APIs. This decision may
> > also determine if we should abandon ds/trace2-tolerate-failed-timestamps or
> > leave the existing behavior as-is.
> 
> I think having APIs that return errors rather than dying on allocation
> failures or overflow is reasonable. The xdiff and reftable code already have
> something similar. I'm not sure "safe" is a good description for those APIs
> though as it does not describe how they differ from the existing APIs.
> Instead of talking about safety I'd rather the documentation talked about
> returning errors on failure and the function naming somehow reflected that.

Yeah, I don't love the term "safe" here for two reasons:

  1. It implies the normal strbuf functions aren't safe. The flaw here
     is the "malloc failure is fatal" scheme, but that's just fine and
     shared with most of the rest of our code. I think we usually
     reserve safe/unsafe for things that you should be extra careful of
     using (like refs_resolve_ref_unsafe, or the unsafe hash algos).

  2. There are many types of safety, and this is just implementing one
     of them. ;) In particular, for the use case in the trace code we're
     looking at, I'd expect that we would want to avoid calling malloc()
     at all, because it relies on locks. So this new code would not be
     safe to call from a signal handler, for example.

So what I had imagined when seeing the initial subject lines was not strbufs who report malloc errors, but rather strbuf-like functions that operate on a fixed-size buffer (with either a static max-size like 4k, or perhaps a per-variable max-size recorded in the struct).

You can still run into errors, of course; we might run out of room in the buffer. But we'd see those cases deterministically for a given input, rather than occasionally when races or other external factors cause malloc to unexpectedly fail.

> For the strbuf API having to check for failure on every function call does
> not sound attractive, I think having a sticky error bit like the stdio
> functions so that one can build a string and check there have been no
> failures once just before using it would be a nicer approach.

Agreed. I think that is a good approach for the static_strbuf idea above, too.

-Peff