On Sat, Jun 8, 2013 at 9:11 PM, René Scharfe <rene.scharfe@xxxxxxxxxxxxxx> wrote: > Am 08.06.2013 19:27, schrieb Felipe Contreras: > >> On Sat, Jun 8, 2013 at 12:22 PM, René Scharfe >> <rene.scharfe@xxxxxxxxxxxxxx> wrote: >> >>> Let's find and fix those leaks by freeing memory in the right places. >>> Freeing memory just in case in places where we can show that no leak is >>> triggered by our test suite doesn't help. >> >> >> It helps; it prevents leaks. The real culprit is the bogus API, but I >> don't see that changing anytime soon, so there are two options when >> somebody makes a mistake the API allows; leak or don't leak. And you >> seem to prefer the leak, even though it provides absolutely no >> advantage. > > It covers up bugs, It doesn't. I thought you already silently agreed that nobody would ever find that leak, as they haven't found the hundreds of leaks that plague Git's code. > What would be a better API? Making discard_index free the array is a good > first step; what else is bogus? 'initialized' for starters; it should be renamed to 'loaded' or removed, but removing it would require many more changes to make sure we don't load twice. Also, when loading cache entries, it might make sense to check if there's already entries that have not been previously discarded properly. In the meantime, just in case, the only sane thing to do is free the entries rather than leak. That being said I'm not interested in this patch any more. The patch is good yet after three tries and countless arguments it's still not applied, nor is there any sign of getting there. -- Felipe Contreras -- 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