On Tue, Mar 6, 2018 at 5:13 PM, Lars Schneider <larsxschneider@xxxxxxxxx> wrote: >> On 06 Mar 2018, at 21:42, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> On Sun, Mar 4, 2018 at 3:14 PM, <lars.schneider@xxxxxxxxxxxx> wrote: >>> + return xstrdup_toupper(value); >> >> xstrdup_toupper() allocates memory... >> >>> + const char *working_tree_encoding; /* Supported encoding or default encoding if NULL */ >> >> ...which is assigned to 'const char *'... >> >>> + ca->working_tree_encoding = git_path_check_encoding(ccheck + 5); >> >> ...by this code, and eventually leaked. >> >> It's too bad it isn't cleaned up (freed), but looking at the callers, >> fixing this leak would be mildly noisy (though not particularly >> invasive). How much do we care about this leak? > > Hmm. You are right. That was previously handled by the encoding struct > linked list that I removed in this iteration. I forgot about that aspect :/ > I don't like it leaking. I think I would like to reintroduce the linked > list. This way every encoding is only once in memory. What do you think? It's subjective, but I find the use of a linked-list just for the purpose of not leaking these strings unnecessarily confusing. If I was doing it, I'd probably add a conv_attrs_free() function and call it from each function allocates a 'struct conv_attrs' (including calling it before early returns -- which prompted my earlier comment about it being a "mildly noisy" fix).
