[phobos] Bug 3763 (How to fix readlnImpl())

[Andrei Alexandrescu]

David Simcha wrote:
> 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.

Could you please give an example?

> 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?

Appender should have had that flag for a while now. Would be great if 
you added it.


Thanks,

Andrei