[phobos] Bug 3763 (How to fix readlnImpl())
David Simcha
dsimcha at gmail.com
Wed Feb 3 17:49:21 PST 2010
I recently filed bug 3673
(http://d.puremagic.com/issues/show_bug.cgi?id=3763) and started looking
at possible fixes. This is a bug in std.stdio.readlnImpl(). However,
the more I think about it the more I think the function needs to be
completely rethought, not just patched. I'm not sure if this is a high
priority given that it has nothing to do with the language spec and TDPL
but it's a pretty embarrassing quality of implementation issue.
Anyhow, readlnImpl() takes a ref char[] and tries to recycle the memory
to read in another line. In doing so, it queries GC.capacity for the
relevant memory block and resizes the array to the size of the memory
block. This is arguably unsafe in the general case because, if someone
passes in a slice that starts at the beginning of a GC block to use as
the buffer, everything else in the same GC block can get overwritten.
This massively violates the principle of least surprise. On the other
hand, when encapsulated in the higher level API of byLine(), it's both
safe and efficient.
A second issue is that the GCC_IO and GENERIC_IO versions use the
following idiom:
buf.length = 0;
// append to buf
This, IIRC, isn't going to result in the memory block being recycled
anymore once Steve's changes to the way arrays work are checked in. The
best thing to do IMHO is to solve the general case of this problem by
adding a useExistingBuffer() function to std.array.Appender. This would
take an existing array of elements that must be mutable, set the
Appender's capacity field to the array's length (NOT its capacity as
reported by the GC, to avoid stomping on arrays that are not owned by
the current scope), and then work like it does now.
Thoughts?
More information about the phobos
mailing list