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. Look: 6 files changed, 15 insertions(+), 5 deletions(-) you added ten unnecessary lines, and you made the interface harder to use. What was the gain here? > Fix read_old_data possible leaks in case of errors, since even when no > data has been read, the strbufs may have grown to prepare the reads. > strbuf_release must be called on them. That's a separate error, and quite frankly, the best approach to that is likely to instead of breaking strbuf_readlink(), just make the S_IFREG() case release it. I'd suggest that strbuf_read_file() should probably also do a strbuf_release() if it returns a negative error value, but that's a separate issue (and still leaves "read_old_data()" having to release things, since read_old_data() wants to see exactly st_size bytes. Although I suspect we might want to change that, and just make it test for negative too). Linus -- 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
