diff options
Diffstat (limited to 'src/libstore')
| -rw-r--r-- | src/libstore/build.cc | 64 | ||||
| -rw-r--r-- | src/libstore/gc.cc | 4 | ||||
| -rw-r--r-- | src/libstore/local-store.cc | 5 | ||||
| -rw-r--r-- | src/libstore/pathlocks.cc | 2 | ||||
| -rw-r--r-- | src/libstore/remote-store.cc | 2 |
5 files changed, 30 insertions, 47 deletions
diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 539d0b21b278..08f44b392b00 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -415,18 +415,6 @@ static void commonChildInit(Pipe & logPipe) } -/* Convert a string list to an array of char pointers. Careful: the - string list should outlive the array. */ -const char * * strings2CharPtrs(const Strings & ss) -{ - const char * * arr = new const char * [ss.size() + 1]; - const char * * p = arr; - foreach (Strings::const_iterator, i, ss) *p++ = i->c_str(); - *p = 0; - return arr; -} - - ////////////////////////////////////////////////////////////////////// @@ -816,8 +804,8 @@ private: /* Start building a derivation. */ void startBuilder(); - /* Initialise the builder's process. */ - void initChild(); + /* Run the builder's process. */ + void runChild(); friend int childEntry(void *); @@ -1914,9 +1902,11 @@ void DerivationGoal::startBuilder() builderOut.create(); /* Fork a child to build the package. */ + ProcessOptions options; + options.allowVfork = !buildUser.enabled(); pid = startProcess([&]() { - initChild(); - }); + runChild(); + }, options); /* parent */ pid.setSeparatePG(true); @@ -1936,7 +1926,7 @@ void DerivationGoal::startBuilder() } -void DerivationGoal::initChild() +void DerivationGoal::runChild() { /* Warning: in the child we should absolutely not make any SQLite calls! */ @@ -1986,9 +1976,11 @@ void DerivationGoal::initChild() /* Set the hostname etc. to fixed values. */ char hostname[] = "localhost"; - sethostname(hostname, sizeof(hostname)); + if (sethostname(hostname, sizeof(hostname)) == -1) + throw SysError("cannot set host name"); char domainname[] = "(none)"; // kernel default - setdomainname(domainname, sizeof(domainname)); + if (setdomainname(domainname, sizeof(domainname)) == -1) + throw SysError("cannot set domain name"); /* Make all filesystems private. This is necessary because subtrees may have been mounted as "shared" @@ -2122,11 +2114,7 @@ void DerivationGoal::initChild() Strings envStrs; foreach (Environment::const_iterator, i, env) envStrs.push_back(rewriteHashes(i->first + "=" + i->second, rewritesToTmp)); - const char * * envArr = strings2CharPtrs(envStrs); - - Path program = drv.builder.c_str(); - std::vector<const char *> args; /* careful with c_str()! */ - string user; /* must be here for its c_str()! */ + auto envArr = stringsToCharPtrs(envStrs); /* If we are running in `build-users' mode, then switch to the user we allocated above. Make sure that we drop all root @@ -2135,8 +2123,6 @@ void DerivationGoal::initChild() setuid() when run as root sets the real, effective and saved UIDs. */ if (buildUser.enabled()) { - printMsg(lvlChatty, format("switching to user ‘%1%’") % buildUser.getUser()); - if (setgroups(0, 0) == -1) throw SysError("cannot clear the set of supplementary groups"); @@ -2152,29 +2138,25 @@ void DerivationGoal::initChild() } /* Fill in the arguments. */ + Strings args; string builderBasename = baseNameOf(drv.builder); - args.push_back(builderBasename.c_str()); - foreach (Strings::iterator, i, drv.args) { - auto re = rewriteHashes(*i, rewritesToTmp); - auto cstr = new char[re.length()+1]; - std::strcpy(cstr, re.c_str()); - - args.push_back(cstr); - } - args.push_back(0); + args.push_back(builderBasename); + foreach (Strings::iterator, i, drv.args) + args.push_back(rewriteHashes(*i, rewritesToTmp)); + auto argArr = stringsToCharPtrs(args); restoreSIGPIPE(); /* Indicate that we managed to set up the build environment. */ - writeToStderr("\n"); + writeFull(STDERR_FILENO, "\n"); /* Execute the program. This should not return. */ - execve(program.c_str(), (char * *) &args[0], (char * *) envArr); + execve(drv.builder.c_str(), (char * *) &argArr[0], (char * *) &envArr[0]); throw SysError(format("executing ‘%1%’") % drv.builder); } catch (std::exception & e) { - writeToStderr("while setting up the build environment: " + string(e.what()) + "\n"); + writeFull(STDERR_FILENO, "while setting up the build environment: " + string(e.what()) + "\n"); _exit(1); } } @@ -2487,7 +2469,7 @@ void DerivationGoal::handleChildOutput(int fd, const string & data) BZ2_bzWrite(&err, bzLogFile, (unsigned char *) data.data(), data.size()); if (err != BZ_OK) throw Error(format("cannot write to compressed log file (BZip2 error = %1%)") % err); } else if (fdLogFile != -1) - writeFull(fdLogFile, (unsigned char *) data.data(), data.size()); + writeFull(fdLogFile, data); } if (hook && fd == hook->fromHook.readSide) @@ -2797,7 +2779,7 @@ void SubstitutionGoal::tryToRun() args.push_back("--substitute"); args.push_back(storePath); args.push_back(destPath); - const char * * argArr = strings2CharPtrs(args); + auto argArr = stringsToCharPtrs(args); /* Fork the substitute program. */ pid = startProcess([&]() { @@ -2807,7 +2789,7 @@ void SubstitutionGoal::tryToRun() if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1) throw SysError("cannot dup output pipe into stdout"); - execv(sub.c_str(), (char * *) argArr); + execv(sub.c_str(), (char * *) &argArr[0]); throw SysError(format("executing ‘%1%’") % sub); }); diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 49cb11d23290..7959a592d987 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -191,7 +191,7 @@ void LocalStore::addTempRoot(const Path & path) lockFile(fdTempRoots, ltWrite, true); string s = path + '\0'; - writeFull(fdTempRoots, (const unsigned char *) s.data(), s.size()); + writeFull(fdTempRoots, s); /* Downgrade to a read lock. */ debug(format("downgrading to read lock on ‘%1%’") % fnTempRoots); @@ -231,7 +231,7 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds) if (lockFile(*fd, ltWrite, false)) { printMsg(lvlError, format("removing stale temporary roots file ‘%1%’") % path); unlink(path.c_str()); - writeFull(*fd, (const unsigned char *) "d", 1); + writeFull(*fd, "d"); continue; } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index fc48c0405650..3ad80bc4e6f4 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -358,7 +358,8 @@ LocalStore::~LocalStore() i->second.to.close(); i->second.from.close(); i->second.error.close(); - i->second.pid.wait(true); + if (i->second.pid != -1) + i->second.pid.wait(true); } } catch (...) { ignoreException(); @@ -498,7 +499,7 @@ void LocalStore::makeStoreWritable() if (unshare(CLONE_NEWNS) == -1) throw SysError("setting up a private mount namespace"); - if (mount(0, settings.nixStore.c_str(), 0, MS_REMOUNT | MS_BIND, 0) == -1) + if (mount(0, settings.nixStore.c_str(), "none", MS_REMOUNT | MS_BIND, 0) == -1) throw SysError(format("remounting %1% writable") % settings.nixStore); } #endif diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc index f26684afacb9..9db37e8f9aaa 100644 --- a/src/libstore/pathlocks.cc +++ b/src/libstore/pathlocks.cc @@ -33,7 +33,7 @@ void deleteLockFile(const Path & path, int fd) other processes waiting on this lock that the lock is stale (deleted). */ unlink(path.c_str()); - writeFull(fd, (const unsigned char *) "d", 1); + writeFull(fd, "d"); /* Note that the result of unlink() is ignored; removing the lock file is an optimisation, not a necessity. */ } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index cabde051bd02..d08913246321 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -110,7 +110,7 @@ void RemoteStore::connectToDaemon() applications... */ AutoCloseFD fdPrevDir = open(".", O_RDONLY); if (fdPrevDir == -1) throw SysError("couldn't open current directory"); - chdir(dirOf(socketPath).c_str()); + if (chdir(dirOf(socketPath).c_str()) == -1) throw SysError(format("couldn't change to directory of ‘%1%’") % socketPath); Path socketPathRel = "./" + baseNameOf(socketPath); struct sockaddr_un addr; |