git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] strbuf: release memory on read error in strbuf_read_once()
@ 2017-12-07 20:51 René Scharfe
  2017-12-07 21:28 ` Jeff King
  0 siblings, 1 reply; 2+ messages in thread
From: René Scharfe @ 2017-12-07 20:51 UTC (permalink / raw)
  To: Git List; +Cc: Stefan Beller, Junio C Hamano

If other strbuf add functions cause the first allocation and
subsequently encounter an error then they release the memory, restoring
the pristine state of the strbuf.  That simplifies error handling for
callers.

Do the same in strbuf_read_once(), and do it also in case no bytes were
read -- which may or may not be an error as well, depending on the
caller.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 strbuf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 323c49ceb3..ac5a7ab62d 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -386,12 +386,15 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
 
 ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)
 {
+	size_t oldalloc = sb->alloc;
 	ssize_t cnt;
 
 	strbuf_grow(sb, hint ? hint : 8192);
 	cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
 	if (cnt > 0)
 		strbuf_setlen(sb, sb->len + cnt);
+	else if (oldalloc == 0)
+		strbuf_release(sb);
 	return cnt;
 }
 
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] strbuf: release memory on read error in strbuf_read_once()
  2017-12-07 20:51 [PATCH] strbuf: release memory on read error in strbuf_read_once() René Scharfe
@ 2017-12-07 21:28 ` Jeff King
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff King @ 2017-12-07 21:28 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Stefan Beller, Junio C Hamano

On Thu, Dec 07, 2017 at 09:51:26PM +0100, René Scharfe wrote:

> If other strbuf add functions cause the first allocation and
> subsequently encounter an error then they release the memory, restoring
> the pristine state of the strbuf.  That simplifies error handling for
> callers.
> 
> Do the same in strbuf_read_once(), and do it also in case no bytes were
> read -- which may or may not be an error as well, depending on the
> caller.

Makes sense, and the patch is delightfully simple.

For the "0" case nobody should be negatively impacted by dropping the
allocation, as they get a sane 0-length string from the slopbuf (and
anybody who relies on sb->buf being allocated without calling detach or
similar is already doing it wrong).

-Peff

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-12-07 21:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-07 20:51 [PATCH] strbuf: release memory on read error in strbuf_read_once() René Scharfe
2017-12-07 21:28 ` Jeff King

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).