[llvm] r298513 - Make home_directory look in the password database in addition to $HOME.

[Zachary Turner via llvm-commits]

Pushed in 298514.  Feel free to let me know if there is still something
else I can do to make it better.

On Wed, Mar 22, 2017 at 9:25 AM Zachary Turner <zturner at google.com> wrote:

> It's a reasonable concern, I'll fix up the test accordingly and push an
> update shortly.
>
> On Wed, Mar 22, 2017 at 9:20 AM Stephan Bergmann <sbergman at redhat.com>
> wrote:
>
> On 03/22/2017 05:12 PM, Zachary Turner wrote:
> > I wasn't aware people do this.  If that's the case, then perhaps I could
> > simply change the test to get the value from the password database
> > inside the test, then unset HOME, then confirm that it returns the same
> > thing that it says in the password database?
> >
> > It sounds like you don't disagree with the functionality change in
> > home_directory(), but just the way the test is implemented, right?
>
> Right.  (And I myself don't routinely build with such a distorted
> environment, just wondered whether this is a breaking point waiting to
> happen eventually.)
>
> > On Wed, Mar 22, 2017 at 9:02 AM Stephan Bergmann <sbergman at redhat.com
> > <mailto:sbergman at redhat.com>> wrote:
> >
> >     On 03/22/2017 04:24 PM, Zachary Turner via llvm-commits wrote:
> >     > Author: zturner
> >     > Date: Wed Mar 22 10:24:59 2017
> >     > New Revision: 298513
> >     >
> >     > URL: http://llvm.org/viewvc/llvm-project?rev=298513&view=rev
> >     > Log:
> >     > Make home_directory look in the password database in addition to
> >     $HOME.
> >     >
> >     > This is something of an edge case, but when the $HOME environment
> >     > variable is not set, we can still look in the password database
> >     > to get the current user's home directory.
> >     >
> >     > Added a test for this by getting the value of $HOME, then unsetting
> >     > it, then calling home_directory() and verifying that it succeeds
> >     > and that the value is the same as what we originally read from
> >     > the environment.
> >
> >     Why should builds not be supported which are done in an environment
> >     where HOME deliberately points somewhere other than the user's home
> >     directory?
> >
> >     > Modified:
> >     >     llvm/trunk/lib/Support/Unix/Path.inc
> >     >     llvm/trunk/unittests/Support/Path.cpp
> >     >
> >     > Modified: llvm/trunk/lib/Support/Unix/Path.inc
> >     > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=298513&r1=298512&r2=298513&view=diff
> >     >
> >
>  ==============================================================================
> >     > --- llvm/trunk/lib/Support/Unix/Path.inc (original)
> >     > +++ llvm/trunk/lib/Support/Unix/Path.inc Wed Mar 22 10:24:59 2017
> >     > @@ -920,12 +920,18 @@ std::error_code real_path(const Twine &p
> >     >  namespace path {
> >     >
> >     >  bool home_directory(SmallVectorImpl<char> &result) {
> >     > -  if (char *RequestedDir = getenv("HOME")) {
> >     > -    result.clear();
> >     > -    result.append(RequestedDir, RequestedDir +
> strlen(RequestedDir));
> >     > -    return true;
> >     > +  char *RequestedDir = getenv("HOME");
> >     > +  if (!RequestedDir) {
> >     > +    struct passwd *pw = getpwuid(getuid());
> >     > +    if (pw && pw->pw_dir)
> >     > +      RequestedDir = pw->pw_dir;
> >     >    }
> >     > -  return false;
> >     > +  if (!RequestedDir)
> >     > +    return false;
> >     > +
> >     > +  result.clear();
> >     > +  result.append(RequestedDir, RequestedDir +
> strlen(RequestedDir));
> >     > +  return true;
> >     >  }
> >     >
> >     >  static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char>
> >     &Result) {
> >     >
> >     > Modified: llvm/trunk/unittests/Support/Path.cpp
> >     > URL:
> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=298513&r1=298512&r2=298513&view=diff
> >     >
> >
>  ==============================================================================
> >     > --- llvm/trunk/unittests/Support/Path.cpp (original)
> >     > +++ llvm/trunk/unittests/Support/Path.cpp Wed Mar 22 10:24:59 2017
> >     > @@ -328,6 +328,26 @@ TEST(Support, HomeDirectory) {
> >     >    }
> >     >  }
> >     >
> >     > +#ifndef LLVM_ON_WIN32
> >     > +TEST(Support, HomeDirectoryWithNoEnv) {
> >     > +  std::string Original;
> >     > +  char const *path = ::getenv("HOME");
> >     > +  // Don't try to test if we don't have something to compare
> against.
> >     > +  if (!path)
> >     > +    return;
> >     > +  Original = path;
> >     > +  ::unsetenv("HOME");
> >     > +
> >     > +  SmallString<128> HomeDir;
> >     > +  auto status = path::home_directory(HomeDir);
> >     > +  EXPECT_TRUE(status);
> >     > +  EXPECT_EQ(Original, HomeDir);
> >     > +
> >     > +  // Now put the original environment variable back
> >     > +  ::setenv("HOME", Original.c_str(), 1);
> >     > +}
> >     > +#endif
> >     > +
> >     >  TEST(Support, UserCacheDirectory) {
> >     >    SmallString<13> CacheDir;
> >     >    SmallString<20> CacheDir2;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170322/a8d5f7ab/attachment.html>