From c89a3d536891da84403b025c70f1ae225faa0eb2 Mon Sep 17 00:00:00 2001 From: Will Dietz Date: Thu, 1 Mar 2018 15:00:58 -0600 Subject: [PATCH 1/2] don't allocate large buffers on the stack --- src/libstore/build.cc | 6 +++--- src/libutil/archive.cc | 18 +++++++++--------- src/libutil/hash.cc | 6 +++--- src/libutil/util.cc | 23 ++++++++++++----------- src/nix-daemon/nix-daemon.cc | 6 +++--- 5 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1d611ffba..b33649e6b 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4156,8 +4156,8 @@ void Worker::waitForInput() set fds2(j->fds); for (auto & k : fds2) { if (FD_ISSET(k, &fds)) { - unsigned char buffer[4096]; - ssize_t rd = read(k, buffer, sizeof(buffer)); + std::vector buffer(4096); + ssize_t rd = read(k, buffer.data(), buffer.size()); if (rd == -1) { if (errno != EINTR) throw SysError(format("reading from %1%") @@ -4169,7 +4169,7 @@ void Worker::waitForInput() } else { printMsg(lvlVomit, format("%1%: read %2% bytes") % goal->getName() % rd); - string data((char *) buffer, rd); + string data((char *) buffer.data(), rd); j->lastOutput = after; goal->handleChildOutput(k, data); } diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index f71229d8f..59be91c5c 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -40,14 +40,14 @@ static void dumpContents(const Path & path, size_t size, AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (!fd) throw SysError(format("opening file '%1%'") % path); - unsigned char buf[65536]; + std::vector buf(65536); size_t left = size; while (left > 0) { - size_t n = left > sizeof(buf) ? sizeof(buf) : left; - readFull(fd.get(), buf, n); + size_t n = left > buf.size() ? buf.size() : left; + readFull(fd.get(), buf.data(), n); left -= n; - sink(buf, n); + sink(buf.data(), n); } writePadding(size, sink); @@ -146,14 +146,14 @@ static void parseContents(ParseSink & sink, Source & source, const Path & path) sink.preallocateContents(size); unsigned long long left = size; - unsigned char buf[65536]; + std::vector buf(65536); while (left) { checkInterrupt(); - unsigned int n = sizeof(buf); - if ((unsigned long long) n > left) n = left; - source(buf, n); - sink.receiveContents(buf, n); + auto n = buf.size(); + if ((unsigned long long)n > left) n = left; + source(buf.data(), n); + sink.receiveContents(buf.data(), n); left -= n; } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 75e476755..8267481b8 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -256,12 +256,12 @@ Hash hashFile(HashType ht, const Path & path) AutoCloseFD fd = open(path.c_str(), O_RDONLY | O_CLOEXEC); if (!fd) throw SysError(format("opening file '%1%'") % path); - unsigned char buf[8192]; + std::vector buf(8192); ssize_t n; - while ((n = read(fd.get(), buf, sizeof(buf)))) { + while ((n = read(fd.get(), buf.data(), buf.size()))) { checkInterrupt(); if (n == -1) throw SysError(format("reading file '%1%'") % path); - update(ht, ctx, buf, n); + update(ht, ctx, buf.data(), n); } finish(ht, ctx, hash.hash); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 2391e14a9..2c4705f6b 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -229,16 +229,17 @@ bool pathExists(const Path & path) Path readLink(const Path & path) { checkInterrupt(); + std::vector buf; for (ssize_t bufSize = PATH_MAX/4; true; bufSize += bufSize/2) { - char buf[bufSize]; - ssize_t rlSize = readlink(path.c_str(), buf, bufSize); + buf.resize(bufSize); + ssize_t rlSize = readlink(path.c_str(), buf.data(), bufSize); if (rlSize == -1) if (errno == EINVAL) throw Error("'%1%' is not a symlink", path); else throw SysError("reading symbolic link '%1%'", path); else if (rlSize < bufSize) - return string(buf, rlSize); + return string(buf.data(), rlSize); } } @@ -293,10 +294,10 @@ string readFile(int fd) if (fstat(fd, &st) == -1) throw SysError("statting file"); - auto buf = std::make_unique(st.st_size); - readFull(fd, buf.get(), st.st_size); + std::vector buf(st.st_size); + readFull(fd, buf.data(), st.st_size); - return string((char *) buf.get(), st.st_size); + return string((char *) buf.data(), st.st_size); } @@ -438,10 +439,10 @@ Path createTempDir(const Path & tmpRoot, const Path & prefix, static Lazy getHome2([]() { Path homeDir = getEnv("HOME"); if (homeDir.empty()) { - char buf[16384]; + std::vector buf(16384); struct passwd pwbuf; struct passwd * pw; - if (getpwuid_r(getuid(), &pwbuf, buf, sizeof(buf), &pw) != 0 + if (getpwuid_r(getuid(), &pwbuf, buf.data(), buf.size(), &pw) != 0 || !pw || !pw->pw_dir || !pw->pw_dir[0]) throw Error("cannot determine user's home directory"); homeDir = pw->pw_dir; @@ -569,16 +570,16 @@ void writeFull(int fd, const string & s, bool allowInterrupts) string drainFD(int fd) { string result; - unsigned char buffer[4096]; + std::vector buffer(4096); while (1) { checkInterrupt(); - ssize_t rd = read(fd, buffer, sizeof buffer); + ssize_t rd = read(fd, buffer.data(), buffer.size()); if (rd == -1) { if (errno != EINTR) throw SysError("reading from file"); } else if (rd == 0) break; - else result.append((char *) buffer, rd); + else result.append((char *) buffer.data(), rd); } return result; } diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 890bffa19..c3b94614a 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -37,13 +37,13 @@ using namespace nix; static ssize_t splice(int fd_in, void *off_in, int fd_out, void *off_out, size_t len, unsigned int flags) { /* We ignore most parameters, we just have them for conformance with the linux syscall */ - char buf[8192]; - auto read_count = read(fd_in, buf, sizeof(buf)); + std::vector buf(8192); + auto read_count = read(fd_in, buf.data(), buf.size()); if (read_count == -1) return read_count; auto write_count = decltype(read_count)(0); while (write_count < read_count) { - auto res = write(fd_out, buf + write_count, read_count - write_count); + auto res = write(fd_out, buf.data() + write_count, read_count - write_count); if (res == -1) return res; write_count += res; From 6b9a03f5d878ae434b54bb883b51e28082dc30b3 Mon Sep 17 00:00:00 2001 From: Will Dietz Date: Thu, 1 Mar 2018 18:58:41 -0600 Subject: [PATCH 2/2] hoist vector out of loop just in case --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index b33649e6b..a1654917d 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4154,9 +4154,9 @@ void Worker::waitForInput() assert(goal); set fds2(j->fds); + std::vector buffer(4096); for (auto & k : fds2) { if (FD_ISSET(k, &fds)) { - std::vector buffer(4096); ssize_t rd = read(k, buffer.data(), buffer.size()); if (rd == -1) { if (errno != EINTR)