On Tue, Mar 6, 2018 at 5:13 PM, Lars Schneider <larsxschnei...@gmail.com> wrote: >> On 06 Mar 2018, at 21:42, Eric Sunshine <sunsh...@sunshineco.com> wrote: >> On Sun, Mar 4, 2018 at 3:14 PM, <lars.schnei...@autodesk.com> 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).
