{"thread":{"id":"47401","subject":"[PATCH] strbuf: release memory on read error in strbuf_read_once()","startedAt":"2017-12-07T20:53:06Z","lastAt":"2017-12-07T21:29:10Z","messageCount":2,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"334365","messageId":"6a39662f-8ff2-c314-5348-4189737a6853@web.de","threadId":"47401","inReplyTo":null,"subject":"[PATCH] strbuf: release memory on read error in strbuf_read_once()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-12-07T20:51:26Z","receivedAt":"2017-12-07T20:53:06Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"If other strbuf add functions cause the first allocation and\nsubsequently encounter an error then they release the memory, restoring\nthe pristine state of the strbuf.  That simplifies error handling for\ncallers.\n\nDo the same in strbuf_read_once(), and do it also in case no bytes were\nread -- which may or may not be an error as well, depending on the\ncaller.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n strbuf.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex 323c49ceb3..ac5a7ab62d 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -386,12 +386,15 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)\n \n ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n {\n+\tsize_t oldalloc = sb->alloc;\n \tssize_t cnt;\n \n \tstrbuf_grow(sb, hint ? hint : 8192);\n \tcnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);\n \tif (cnt > 0)\n \t\tstrbuf_setlen(sb, sb->len + cnt);\n+\telse if (oldalloc == 0)\n+\t\tstrbuf_release(sb);\n \treturn cnt;\n }\n \n-- \n2.15.1\n"},{"id":"334377","messageId":"20171207212855.GE12850@sigill.intra.peff.net","threadId":"47401","inReplyTo":"6a39662f-8ff2-c314-5348-4189737a6853@web.de","subject":"Re: [PATCH] strbuf: release memory on read error in strbuf_read_once()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-07T21:28:55Z","receivedAt":"2017-12-07T21:29:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 07, 2017 at 09:51:26PM +0100, René Scharfe wrote:\n\n> If other strbuf add functions cause the first allocation and\n> subsequently encounter an error then they release the memory, restoring\n> the pristine state of the strbuf.  That simplifies error handling for\n> callers.\n> \n> Do the same in strbuf_read_once(), and do it also in case no bytes were\n> read -- which may or may not be an error as well, depending on the\n> caller.\n\nMakes sense, and the patch is delightfully simple.\n\nFor the \"0\" case nobody should be negatively impacted by dropping the\nallocation, as they get a sane 0-length string from the slopbuf (and\nanybody who relies on sb->buf being allocated without calling detach or\nsimilar is already doing it wrong).\n\n-Peff\n"}]}