C Util API: Fix leak of demangled error name#14567
Conversation
src/libutil-c/nix_api_util.cc
Outdated
| if (demangled) { | ||
| context->name = demangled; | ||
| // todo: free(demangled); | ||
| free((void*)demangled); |
There was a problem hiding this comment.
I don't know the exact details, but my intuition is that the damangled string's ownership is somewhat stdlib specific. Does this work with both libc++ (we have an llvm build via nix-everything-llvm flake output) and libstdc++?
There was a problem hiding this comment.
Got it, I was just using the "default"/non-suffixed packages. I just ran it again with the *-llvm version and it looks to be working there too. I'm not very familiar with the repo so let me know if there's other things I should test
There was a problem hiding this comment.
The todo was written by @jlesquembre, maybe he remembers what the reason was?
There was a problem hiding this comment.
I don't remember the reason. Maybe I was thinking the context stores a pointer to the error name, but it's actually copied there, so this free should be safe.
5 participants
Motivation
Fix leak of a demangled error name in C util API. Valgrind was flagging this and I noticed the TODO comment--not sure if there was a specific reason for this originally.
Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.