Pierre Habouzit schrieb:
> On Tue, Dec 23, 2008 at 06:16:01PM +0000, Linus Torvalds wrote:
>>
>> On Tue, 23 Dec 2008, Pierre Habouzit wrote:
>>> when readlink fails, the strbuf shall not be destroyed. It's not how
>>> read_file_or_gitlink works for example.
>> I disagree.
>>
>> This patch just makes things worse. Just leave the "strbuf_release()" in
>> _one_ place.
>
> The "problem" is that the strbuf API usually works that way: functions
> append things to a buffer, or do nothing, but always keep the buffer in
> a state where you can append more stuff to it.
>
> If read_file_or_gitlink or strbuf_readlink destroy the buffer, then you
> break the second expectation people (should) have about the strbuf API.
The "append or do nothing" rule is broken by strbuf_getline(), but I agree
to your reasoning. How about refining this rule a bit to "do your thing
and roll back changes if an error occurs"? I think it's not worth to undo
allocation extensions, but making reverting first time allocations seems
like a good idea. Something like this?
René
PS: only nine lines! ;-)
strbuf.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index bdf4954..6ed0684 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -256,18 +256,21 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
{
size_t res;
+ size_t oldalloc = sb->alloc;
strbuf_grow(sb, size);
res = fread(sb->buf + sb->len, 1, size, f);
- if (res > 0) {
+ if (res > 0)
strbuf_setlen(sb, sb->len + res);
- }
+ else if (res < 0 && oldalloc == 0)
+ strbuf_release(sb);
return res;
}
ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
{
size_t oldlen = sb->len;
+ size_t oldalloc = sb->alloc;
strbuf_grow(sb, hint ? hint : 8192);
for (;;) {
@@ -275,7 +278,10 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
if (cnt < 0) {
- strbuf_setlen(sb, oldlen);
+ if (oldalloc == 0)
+ strbuf_release(sb);
+ else
+ strbuf_setlen(sb, oldlen);
return -1;
}
if (!cnt)
@@ -292,6 +298,8 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)
int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
{
+ size_t oldalloc = sb->alloc;
+
if (hint < 32)
hint = 32;
@@ -311,7 +319,8 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
/* .. the buffer was too small - try again */
hint *= 2;
}
- strbuf_release(sb);
+ if (oldalloc == 0)
+ strbuf_release(sb);
return -1;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
