2 ms·
I only partially disagree with the sentiment that it is "impossible to fix". For the current API that is true, but a fairly minor modification API would make i
by jandrese 2y ago
I only partially disagree with the sentiment that it is "impossible to fix". For the current API that is true, but a fairly minor modification API would make it possible. All getenv() has to do is strdup() the return value before sending it back and leaving it on the programmer to free the memory when they are done with it. This does mean that the programmer will need to call getenv() again if they think the value might change, but I think that is a reasonable tradeoff.
This change would make old programs leak memory every time they call getenv() without the subsequent free(), but since the current version also leaks memory that doesn't seem like a dealbreaker. As an added bonus the new version could be made thread safe by wrapping the strdup() in a mutex and doing similar work on the setenv() side.
- bhawks 2y agoThere exists a ton of working code that does not fiddle with setenv and now would have memory leaks if they don't change their code? Plus I would now need to test if my stdlib requires freeing memory or not because if I try to free on an older libc it is not going to work out well. I don't think the value works out. From a hygiene perspective - freeing the return of another API is an anti pattern. If you need the caller to release objects there should provide a FooLib_bar_destory(bar) or similar.
- jandrese 2y agoYes, it would cause existing programs to leak memory. Most of the time these leaks would be fairly minor, but some programs could leak a lot if they call getenv() inside of a loop for some reason. Personally, if the return object is a basic C type, I'm not a fan of creating a wrapper function to call free(). This is one of those code purity things that I don't think buys you anything.