Fix bug in byLine() with empty files #375
Conversation
|
I wonder if file size might be slightly better? Would only involve touching the file metadata rather than contents. |
|
Oh, also, all those tests that use files without unique names and/or directories are a landmine when it comes to parallel builds/tests. I really want make -j to work reliably. The auto-testers are already doing that since it's critical to throughput. |
|
Good point, hold on. |
|
That won't work with pipes and other special files (there are Unix files that report size 0 yet they have stuff in them). Besides, I'm thinking if someone creates a byLine they're very likely to start actually reading stuff, so warming the buffers seems reasonable. |
|
Good point. Too bad. Failure on distinguish between unknown and none foils a perfectly reasonable plan once again. Anyway, the hardcoded filenames issues still remain |
|
Hardcoded filenames fixed in the last commit. |
| std.file.write("deleteme", "1 2 3"); | ||
| auto f = File("deleteme"); | ||
| auto deleteme = testFilename(); | ||
| scope(exit) collectException(std.file.remove(deleteme)); |
There was a problem hiding this comment.
Is there a particular reason why this uses collectException and none of the other lines like this do? I guess that that's what it was doing before rather than you doing it now, but it's still odd that this one does it and none of the others do.
|
Fails on win32: http://d.puremagic.com/test-results/pull.ghtml?runid=12297 |
|
I pulled your changes down and fixed the stdio.d on windows issues. I didn't address jmdavis' questions. They can be delt with in a separate pull. |
|
Great. Thanks Brad and sorry for moving slow on this! |
Merge remote-tracking branch 'upstream/stable' into merge_stable
byLine() had a bug (assertion failed) with empty files. This is because opening an empty file does not set the end-of-file indicator. The proposed solution issues a pair of getc/ungetc calls only once just before the very first read. This is unlikely to affect performance. If there are better solutions, please chime in.