aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGuillaume Bouchard <guillaume.bouchard@tweag.io>2020-04-29 13:10:03 +0200
committerGuillaume Bouchard <guillaume.bouchard@tweag.io>2020-04-29 14:50:52 +0200
commit5a34a473dd14200ffeff496f0e8e4518f7992765 (patch)
tree759115d621e4f9678fda5f49c5d08cafa53b3d09
parent06849c3090615d85dbba17b674e97dff71bd41bc (diff)
builtins.readFile: do not truncate content
This closes #3026 by allowing `builtins.readFile` to read a file with a wrongly reported file size, for example, files in `/proc` may report a file size of 0. Reading file in `/proc` is not a good enough motivation, however I do think it just makes nix more robust by allowing more file to be read. Especially, I do considerer the previous behavior to be dangerous because nix was previously reading truncated files. Examples of file system which incorrectly report file size may be network file system or dynamic file system (for performance reason, a dynamic file system such as FUSE may generate the content of the file on demand). ``` nix-repl> builtins.readFile "/proc/version" "" ``` With this commit: ``` nix-repl> builtins.readFile "/proc/version" "Linux version 5.6.7 (nixbld@localhost) (gcc version 9.3.0 (GCC)) #1-NixOS SMP Thu Apr 23 08:38:27 UTC 2020\n" ``` Here is a summary of the behavior changes: - If the reported size is smaller, previous implementation was silently returning a truncated file content. The new implementation is returning the correct file content. - If a file had a bigger reported file size, previous implementation was failing with an exception, but the new implementation is returning the correct file content. This change of behavior is coherent with this pull request. Open questions - The behavior is unchanged for correctly reported file size, however performances may vary because it uses the more complex sink interface. Considering that sink is used a lot, I don't think this impacts the performance a lot. - `builtins.readFile` on an infinite file, such as `/dev/random` may fill the memory. - it does not support adding file to store, such as `${/proc/version}`.
-rw-r--r--src/libutil/util.cc9
1 files changed, 1 insertions, 8 deletions
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 615a7656c..d001bc3b7 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -312,14 +312,7 @@ unsigned char getFileType(const Path & path)
string readFile(int fd)
{
- struct stat st;
- if (fstat(fd, &st) == -1)
- throw SysError("statting file");
-
- std::vector<unsigned char> buf(st.st_size);
- readFull(fd, buf.data(), st.st_size);
-
- return string((char *) buf.data(), st.st_size);
+ return drainFD(fd, true);
}