This patch adds alternative way to get the user's home directory.
Details
Diff Detail
Event Timeline
Hello Chandler,
RE: Why is this useful?
It tries to get home_direictory by all ways. It's useful in case when HOME wasn't set (that was in my case).
RE: The documentation for getpwuid specifically suggests that applications should merely inspect the HOME environment variable.
I didn't see any references to HOME variable. (see http://pubs.opengroup.org/onlinepubs/009695399/functions/getpwuid.html)
RE: If this is truly necessary, it should use getpwuid_r to be thread-safe, and it should probably be protected by compatibility macros as I don't know which of the unix variants we support implement this functionality. But maybe it is sufficiently widely available across BSD, Linux, Solaris, Unix, and Mac OS variants....
I have updated my patch (now it uses thread-safe function) but I don't know how to protect the inclusion of pwd.h (HAVE_PWD_H is absent).
Of course I can setup HOME (using getpwuid()) on start but I think that path::home_directory() should do it for me.
Thanks,
Ilia
Hello Chandler,
RE: But what platform is it that we're hypothetically supporting? All the BSD and Linux flavors I know of rely heavily on the HOME environment variable. All the build bots I know of test under that assumption. I'm worried about adding code to support another variant that we have essentially no way of testing and no knowledge of why it was important.
I'm working on OS X but in this case I launch lldb from another process. Perhaps I forgot to pass HOME variable in execve(). I didn't know that lldb (and llvm) uses that variable. I think we should fix it to avoid such problems in the future (or warn that HOME variable isn't set).
Thanks,
Ilia
A few comments:
- Use clang-format or manually follow the rules defined here: http://llvm.org/docs/CodingStandards.html
- You are using a VLA, this is not C++ standard I believe.
- At least add a clear comment on what you are doing and why (explain why getenv("HOME") might fail.
Thanks.
Hello Chandler,
RE: It's really unclear to me that we should support this at all rather. I feel like we're adding complexity we really don't need.
Ok. I don't mind. I had found an error and can fix it on my side.
@joker.eph,@chandlerc, thank you for your time.
Thanks,
Ilia
FWIW, it seems to me that Python is doing it this way: https://github.com/enthought/Python-2.7.3/blob/master/Lib/posixpath.py
As you see my fix does the same thing.
Are you interested in this patch? If so I can fix your remarks