Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(92)

Issue 89550043: code review 89550043: acme: use buffered i/o to write file (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by rsc
Modified:
10 years, 8 months ago
Reviewers:
r
Visibility:
Public.

Description

acme: use buffered i/o to write file Bakul Shah has observed corrupted files being written when acme writes over osxfuse to sshfs to a remote file system. In one example we examined, acme is writing an 0xf03-byte file in two system calls, first an 0x806-byte write and then a 0x6fd-byte write. (0x806 is BUFSIZE/sizeof(Rune); this file has no multibyte UTF-8.) What actually ends up happening is that an 0x806-byte file is written: 0x000-0x6fd contains what should be 0x806-0xf03 0x6fd-0x7fa contains zeros 0x7fa-0x806 contains what should be 0x7fa-0x806 (correct!) The theory is that fuse or sshfs or perhaps the remote file server is mishandling the unaligned writes. acme does not seem to be at fault. Using bio here will make the writes align to 8K boundaries, avoiding the bugs in whatever underlying piece is broken.

Patch Set 1 #

Patch Set 2 : diff -r db0e4f2b8323 https://code.google.com/p/plan9port #

Patch Set 3 : diff -r db0e4f2b8323 https://code.google.com/p/plan9port #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -1 line) Patch
M src/cmd/acme/exec.c View 1 5 chunks +16 lines, -1 line 1 comment Download

Messages

Total messages: 3
rsc
Hello r, I'd like you to review this change to https://code.google.com/p/plan9port
10 years, 8 months ago (2014-04-19 14:09:18 UTC) #1
rsc
*** Submitted as https://code.google.com/p/plan9port/source/detail?r=e0309ab91bfe *** acme: use buffered i/o to write file Bakul Shah has ...
10 years, 8 months ago (2014-04-19 14:09:23 UTC) #2
r
10 years, 8 months ago (2014-04-19 16:47:27 UTC) #3
Message was sent while issue was closed.
https://codereview.appspot.com/89550043/diff/40001/src/cmd/acme/exec.c
File src/cmd/acme/exec.c (right):

https://codereview.appspot.com/89550043/diff/40001/src/cmd/acme/exec.c#newcod...
src/cmd/acme/exec.c:637: Biobuf *b;
acme's source is from another era but i still think this is worth a comment
since it is a workaround for a bug rather than the usual purpose of bio
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b