No scratch for absPath?
#9775
No scratch for absPath?
#9775
Conversation
Ericson2314
force-pushed
the
abs-path-string-view
branch
2 times, most recently
from
fe178aa to
966b3e3
Compare
Ericson2314
force-pushed
the
abs-path-string-view
branch
5 times, most recently
from
561ba93 to
6caad76
Compare
I found a way to avoid the `scratch` variable I just introduced in NixOS#9759, but I am not sure whether this makes this code easier or harder to read 😅.
Ericson2314
force-pushed
the
abs-path-string-view
branch
from
6caad76 to
2512c4a
Compare
|
@tfc I am curious whether you think this |
| return canonPath(path, resolveSymlinks); | ||
| "/", | ||
| path)), | ||
| resolveSymlinks); |
There was a problem hiding this comment.
i see the elegance in point-free programming in general, but the if-branched ersion that we had before was more readable.
| { | ||
| std::string scratch; | ||
| return std::unique_ptr<char, FreeDeleter> { getcwd(NULL, 0) }; |
There was a problem hiding this comment.
is this called so often that it must be really fast and small, so that it is worth spilling this memory management over the code that wants the cwd string?
If no, then please create a function like std::string getCwd() that copies the content and then frees the original string. this way we don't need anyone to know about custom deleters for this purpose.
| free(buf); | ||
| getcwd(buf, sizeof(buf)) | ||
| ? buf | ||
| : throw SysError("cannot get cwd"), |
There was a problem hiding this comment.
can't we encapsulate these two branches and the whole legacy mem management issue simply into a function std::string getCwd() that does it all automagically (with the performance-question yes/no from earler in mind)?
this seems like an unnecessary dance for a function whose concern is making paths absolute.
|
@tfc and I discussed this. Yes I'll stop being a weenie and do @roberth and I further discussed @tfc's caching idea, and @roberth had the code idea that we can wrap |
Motivation
I found a way to avoid the
scratchvariable I just introduced in #9759, but I am not sure whether this makes this code easier or harder to read 😅.Context
See conversation in #9759
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.