[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