Commit d96b6c0d9f7a4fc6e7e28dd92d18e548f00885de | zig

author	Andrew Kelley <andrew@ziglang.org> 2020-03-11 13:06:30-04:00
committer	Andrew Kelley <andrew@ziglang.org> 2020-03-11 13:06:30-04:00
log	d96b6c0d9f7a4fc6e7e28dd92d18e548f00885de
tree	1c2774d9d9ffe279fa1ae330eec529dbdc9cb994
parent	ed13cffca4c42718cc7239faf7aab642ac67ef77
signature	Commit is signed but in an unrecognized format.

fix footguns in File readAll functions

4 files changed, 75 insertions(+), 37 deletions(-)

lib
- std
  - fs.zig
  - fs
    file.zig
  - io
    in_stream.zig
  - os
    test.zig

lib/std/fs.zig+12-21

 @@ -96,6 +96,7 @@ pub fn updateFile(source_path: []const u8, dest_path: []const u8) !PrevStatus {
 /// atime, and mode of the source file so that the next call to `updateFile` will not need a copy.
 /// Returns the previous status of the file before updating.
 /// If any of the directories do not exist for dest_path, they are created.
+/// TODO rework this to integrate with Dir
 pub fn updateFileMode(source_path: []const u8, dest_path: []const u8, mode: ?File.Mode) !PrevStatus {
     const my_cwd = cwd();
 @@ -141,29 +142,25 @@ pub fn updateFileMode(source_path: []const u8, dest_path: []const u8, mode: ?Fil
 /// there is a possibility of power loss or application termination leaving temporary files present
 /// in the same directory as dest_path.
 /// Destination file will have the same mode as the source file.
+/// TODO rework this to integrate with Dir
 pub fn copyFile(source_path: []const u8, dest_path: []const u8) !void {
     var in_file = try cwd().openFile(source_path, .{});
     defer in_file.close();
-    const mode = try in_file.mode();
+    const stat = try in_file.stat();
-    const in_stream = &in_file.inStream().stream;
-    var atomic_file = try AtomicFile.init(dest_path, mode);
+    var atomic_file = try AtomicFile.init(dest_path, stat.mode);
     defer atomic_file.deinit();
-    var buf: [mem.page_size]u8 = undefined;
+    try atomic_file.file.writeFileAll(in_file, .{ .in_len = stat.size });
-    while (true) {
+    return atomic_file.finish();
-        const amt = try in_stream.readFull(buf[0..]);
-        try atomic_file.file.write(buf[0..amt]);
-        if (amt != buf.len) {
-            return atomic_file.finish();
-        }
-    }
 }
-/// Guaranteed to be atomic. However until https://patchwork.kernel.org/patch/9636735/ is
+/// Guaranteed to be atomic.
-/// merged and readily available,
+/// On Linux, until https://patchwork.kernel.org/patch/9636735/ is merged and readily available,
 /// there is a possibility of power loss or application termination leaving temporary files present
+/// in the same directory as dest_path.
+/// TODO rework this to integrate with Dir
 pub fn copyFileMode(source_path: []const u8, dest_path: []const u8, mode: File.Mode) !void {
     var in_file = try cwd().openFile(source_path, .{});
     defer in_file.close();
 @@ -171,14 +168,8 @@ pub fn copyFileMode(source_path: []const u8, dest_path: []const u8, mode: File.M
     var atomic_file = try AtomicFile.init(dest_path, mode);
     defer atomic_file.deinit();
-    var buf: [mem.page_size * 6]u8 = undefined;
+    try atomic_file.file.writeFileAll(in_file, .{});
-    while (true) {
+    return atomic_file.finish();
-        const amt = try in_file.read(buf[0..]);
-        try atomic_file.file.write(buf[0..amt]);
-        if (amt != buf.len) {
-            return atomic_file.finish();
-        }
-    }
 }
 /// TODO update this API to avoid a getrandom syscall for every operation. It

lib/std/fs/file.zig+33-9

 @@ -250,11 +250,16 @@ pub const File = struct {
         }
     }
-    pub fn readAll(self: File, buffer: []u8) ReadError!void {
+    /// Returns the number of bytes read. If the number read is smaller than `buffer.len`, it
+    /// means the file reached the end. Reaching the end of a file is not an error condition.
+    pub fn readAll(self: File, buffer: []u8) ReadError!usize {
         var index: usize = 0;
-        while (index < buffer.len) {
+        while (index != buffer.len) {
-            index += try self.read(buffer[index..]);
+            const amt = try self.read(buffer[index..]);
+            if (amt == 0) break;
+            index += amt;
         }
+        return index;
     }
     pub fn pread(self: File, buffer: []u8, offset: u64) PReadError!usize {
 @@ -265,11 +270,16 @@ pub const File = struct {
         }
     }
-    pub fn preadAll(self: File, buffer: []u8, offset: u64) PReadError!void {
+    /// Returns the number of bytes read. If the number read is smaller than `buffer.len`, it
+    /// means the file reached the end. Reaching the end of a file is not an error condition.
+    pub fn preadAll(self: File, buffer: []u8, offset: u64) PReadError!usize {
         var index: usize = 0;
-        while (index < buffer.len) {
+        while (index != buffer.len) {
-            index += try self.pread(buffer[index..], offset + index);
+            const amt = try self.pread(buffer[index..], offset + index);
+            if (amt == 0) break;
+            index += amt;
         }
+        return index;
     }
     pub fn readv(self: File, iovecs: []const os.iovec) ReadError!usize {
 @@ -280,19 +290,27 @@ pub const File = struct {
         }
     }
+    /// Returns the number of bytes read. If the number read is smaller than the total bytes
+    /// from all the buffers, it means the file reached the end. Reaching the end of a file
+    /// is not an error condition.
     /// The `iovecs` parameter is mutable because this function needs to mutate the fields in
     /// order to handle partial reads from the underlying OS layer.
-    pub fn readvAll(self: File, iovecs: []os.iovec) ReadError!void {
+    pub fn readvAll(self: File, iovecs: []os.iovec) ReadError!usize {
         if (iovecs.len == 0) return;
         var i: usize = 0;
+        var off: usize = 0;
         while (true) {
             var amt = try self.readv(iovecs[i..]);
+            var eof = amt == 0;
+            off += amt;
             while (amt >= iovecs[i].iov_len) {
                 amt -= iovecs[i].iov_len;
                 i += 1;
-                if (i >= iovecs.len) return;
+                if (i >= iovecs.len) return off;
+                eof = false;
             }
+            if (eof) return off;
             iovecs[i].iov_base += amt;
             iovecs[i].iov_len -= amt;
         }
 @@ -306,6 +324,9 @@ pub const File = struct {
         }
     }
+    /// Returns the number of bytes read. If the number read is smaller than the total bytes
+    /// from all the buffers, it means the file reached the end. Reaching the end of a file
+    /// is not an error condition.
     /// The `iovecs` parameter is mutable because this function needs to mutate the fields in
     /// order to handle partial reads from the underlying OS layer.
     pub fn preadvAll(self: File, iovecs: []const os.iovec, offset: u64) PReadError!void {
 @@ -315,12 +336,15 @@ pub const File = struct {
         var off: usize = 0;
         while (true) {
             var amt = try self.preadv(iovecs[i..], offset + off);
+            var eof = amt == 0;
             off += amt;
             while (amt >= iovecs[i].iov_len) {
                 amt -= iovecs[i].iov_len;
                 i += 1;
-                if (i >= iovecs.len) return;
+                if (i >= iovecs.len) return off;
+                eof = false;
             }
+            if (eof) return off;
             iovecs[i].iov_base += amt;
             iovecs[i].iov_len -= amt;
         }

lib/std/io/in_stream.zig+1-4

 @@ -28,10 +28,7 @@ pub fn InStream(
             return readFn(self.context, buffer);
         }
-        /// Deprecated: use `readAll`.
+        /// Returns the number of bytes read. If the number read is smaller than `buffer.len`, it
-        pub const readFull = readAll;
-        /// Returns the number of bytes read. If the number read is smaller than buf.len, it
         /// means the stream reached the end. Reaching the end of a stream is not an error
         /// condition.
         pub fn readAll(self: Self, buffer: []u8) Error!usize {

lib/std/os/test.zig+29-3

...	@@ -95,15 +95,41 @@ test "sendfile" {	...	@@ -95,15 +95,41 @@ test "sendfile" {
95	},	95	},
96	};	96	};
97		97
98	var written_buf: [header1.len + header2.len + 10 + trailer1.len + trailer2.len]u8 = undefined;	98	var written_buf: [100]u8 = undefined;
99	try dest_file.writeFileAll(src_file, .{	99	try dest_file.writeFileAll(src_file, .{
100	.in_offset = 1,	100	.in_offset = 1,
101	.in_len = 10,	101	.in_len = 10,
102	.headers_and_trailers = &hdtr,	102	.headers_and_trailers = &hdtr,
103	.header_count = 2,	103	.header_count = 2,
104	});	104	});
105	try dest_file.preadAll(&written_buf, 0);	105	const amt = try dest_file.preadAll(&written_buf, 0);
106	expect(mem.eql(u8, &written_buf, "header1\nsecond header\nine1\nsecontrailer1\nsecond trailer\n"));	106	expect(mem.eql(u8, written_buf[0..amt], "header1\nsecond header\nine1\nsecontrailer1\nsecond trailer\n"));
		107	}
		108
		109	test "fs.copyFile" {
		110	const data = "u6wj+JmdF3qHsFPE BUlH2g4gJCmEz0PP";
		111	const src_file = "tmp_test_copy_file.txt";
		112	const dest_file = "tmp_test_copy_file2.txt";
		113	const dest_file2 = "tmp_test_copy_file3.txt";
		114
		115	try fs.cwd().writeFile(src_file, data);
		116	defer fs.cwd().deleteFile(src_file) catch {};
		117
		118	try fs.copyFile(src_file, dest_file);
		119	defer fs.cwd().deleteFile(dest_file) catch {};
		120
		121	try fs.copyFileMode(src_file, dest_file2, File.default_mode);
		122	defer fs.cwd().deleteFile(dest_file2) catch {};
		123
		124	try expectFileContents(dest_file, data);
		125	try expectFileContents(dest_file2, data);
		126	}
		127
		128	fn expectFileContents(file_path: []const u8, data: []const u8) !void {
		129	const contents = try fs.cwd().readFileAlloc(testing.allocator, file_path, 1000);
		130	defer testing.allocator.free(contents);
		131
		132	testing.expectEqualSlices(u8, data, contents);
107	}	133	}
108		134
109	test "std.Thread.getCurrentId" {	135	test "std.Thread.getCurrentId" {