From: "René Scharfe" <l.s.r@web•de>
To: Jeff King <peff@peff•net>, Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org
Subject: Re: [RFC] cocci: .buf in a strbuf object can never be NULL
Date: Sat, 21 Mar 2026 21:47:18 +0100 [thread overview]
Message-ID: <3e387439-c066-4e45-b28b-43f77c8824d6@web.de> (raw)
In-Reply-To: <20260319233546.GA3632561@coredump.intra.peff.net>
On 3/20/26 12:35 AM, Jeff King wrote:
>
> @@ -669,10 +673,13 @@ int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
> * we can just re-init, but otherwise we should make sure that our
> * length is empty, and that the result is NUL-terminated.
> */
> - if (!sb->buf)
> + if (!buf)
> strbuf_init(sb, 0);
> - else
> - strbuf_reset(sb);
> + else {
> + sb->buf = buf;
> + sb->alloc = alloc;
> + strbuf_reset(&sb);
> + }
> return EOF;
> }
> #else
>
> So I don't know that it makes anything simpler. We have to copy the
> values back into the strbuf either way, and we still have to handle
> restoring the strbuf invariants. Even the strbuf_init() case is still
> needed, because we don't know whether getdelim() just didn't allocate
> (in which case we could leave the strbuf alone) or if it actually ate
> the allocation we passed in (which was just a copy of sb->buf).
And yet this function can turn an empty strbuf into an allocated one
without rolling it back on error, leaving code similar to this silly
example here leaking:
int copy_one_line(FILE *in, FILE *out, int term)
{
struct strbuf sb = STRBUF_INIT;
if (strbuf_getwholeline(&sb, in, term))
return -1;
fwrite(sb.buf, 1, sb.len, out);
strbuf_release(&sb);
return 0;
}
Some strbuf functions restore the original state in such a case by
calling strbuf_release(), strbuf_getwholeline() doesn't. If we are OK
with that then it could be simplified by growing the buffer upfront:
int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
{
ssize_t r;
strbuf_grow(sb, 0);
errno = 0;
r = getdelim(&sb->buf, &sb->alloc, term, fp);
if (r > 0) {
sb->len = r;
return 0;
}
assert(r == -1);
if (errno == ENOMEM)
die("Out of memory, getdelim failed");
strbuf_reset(sb);
return EOF;
}
René
next prev parent reply other threads:[~2026-03-21 20:47 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-19 7:15 [PATCH] rerere: update to modern representation of empty strbufs Junio C Hamano
2026-03-19 7:57 ` Patrick Steinhardt
2026-03-19 22:14 ` [RFC] cocci: .buf in a strbuf object can never be NULL Junio C Hamano
2026-03-19 23:35 ` Jeff King
2026-03-20 1:46 ` Junio C Hamano
2026-03-20 4:18 ` Jeff King
2026-03-20 5:45 ` Junio C Hamano
2026-03-20 5:57 ` Jeff King
2026-03-20 6:06 ` Junio C Hamano
2026-03-20 6:18 ` Jeff King
2026-03-21 13:14 ` René Scharfe
2026-03-21 16:41 ` Jeff King
2026-03-21 20:47 ` René Scharfe [this message]
2026-03-21 21:18 ` Jeff King
2026-03-21 23:41 ` René Scharfe
2026-03-22 1:44 ` Jeff King
2026-03-22 1:22 ` Junio C Hamano
2026-03-22 1:40 ` Jeff King
2026-03-21 16:24 ` Junio C Hamano
2026-03-21 16:39 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3e387439-c066-4e45-b28b-43f77c8824d6@web.de \
--to=l.s.r@web$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox