[erlang-patches] : fread failing to handle newlines gracefully

[Raimo Niskanen]

On Thu, Feb 05, 2009 at 09:35:01AM +0100, Raimo Niskanen wrote:
> On Fri, Jan 30, 2009 at 11:12:23PM +1100, Matthew Palmer wrote:
> > Hi,
> > 
> > I recently fielded a question on Stack Overflow regarding some strange
> > behaviour of fread, with the ~d format spec
> > (http://stackoverflow.com/questions/473327/unexpected-behavior-of-iofread-in-erlang/490023#490023). 
> > Digging deep, it appears to be a bit of a bug in the way that newlines are
> > handled in the format parser, as demonstrated here:
> > 
> > 3> io_lib_fread:fread([], "10 11\n12 13 14\n", "~d").
> > {done,{ok,"\n"}," 1112 13 14\n"}
> > 
> > It's eaten the newline between "11" and "12", which means that on the next
> > pass through it'll read the next number as "1112" instead of "11", which
> > poses some obvious problems.
> > 
> > I've come up with the attached patch to fix the problem.  I'm not very
> > experienced with Erlang yet, so it might not be the cleanest way to do it,
> > but it does definitely fix the problem, and without (apparently) breaking
> > any other common use case.  Comments appreciated.  If there's a test suite I
> > should be adding to as well, I'm more than happy to do that, I just couldn't
> > find one in my source tarball.
> > 
> > - Matt
> 
> Thank you for your effort!
> 
> Your diagnosis and patch are basically right. I have now been digging
> into the matter and it took a while to ensure that. I will polish
> the patch a bit (the code needs some cleanup), and improve it
> for \r\n combinations.
> 
> Unfortunately there are no tests in the source tarball. We are striving
> towards their release but so far large parts of them are too much
> depending on our internal environment to be globally useful...
> 
> I will write tests too. When done I will release a new
> source code patch here.

OK. The patch is appended. I also found and corrected
a bug for e.g io_lib:fread("~s", "\rabc").

> 
> > --- ../erlang/erlang-12.b.3-dfsg/lib/stdlib/src/io_lib_fread.erl	2004-09-14 21:41:00.000000000 +1000
> > +++ ./io_lib_fread.erl	2009-01-30 15:44:57.000000000 +1100
> > @@ -35,9 +35,9 @@
> >      fread_collect(MoreChars, [], Rest, RestFormat, N, Inputs).
> >  
> >  fread_collect([$\r|More], Stack, Rest, RestFormat, N, Inputs) ->
> > -    fread(RestFormat, Rest ++ reverse(Stack), N, Inputs, More);
> > +    fread(RestFormat, Rest ++ reverse(Stack), N, Inputs, [$\r|More]);
> >  fread_collect([$\n|More], Stack, Rest, RestFormat, N, Inputs) ->
> > -    fread(RestFormat, Rest ++ reverse(Stack), N, Inputs, More);
> > +    fread(RestFormat, Rest ++ reverse(Stack), N, Inputs, [$\n|More]);
> >  fread_collect([C|More], Stack, Rest, RestFormat, N, Inputs) ->
> >      fread_collect(More, [C|Stack], Rest, RestFormat, N, Inputs);
> >  fread_collect([], Stack, Rest, RestFormat, N, Inputs) ->
> > @@ -55,8 +55,8 @@
> >  		eof ->
> >  		    fread(RestFormat,eof,N,Inputs,eof);
> >  		_ ->
> > -		    %% Don't forget to count the newline.
> > -		    {more,{More,RestFormat,N+1,Inputs}}
> > +		    %% Don't forget to strip and count the newline.
> > +		    {more,{tl(More),RestFormat,N+1,Inputs}}
> >  	    end;
> >  	Other ->				%An error has occurred
> >  	    {done,Other,More}
> 
> > _______________________________________________
> > erlang-patches mailing list
> > erlang-patches@REDACTED
> > http://www.erlang.org/mailman/listinfo/erlang-patches
> 
> -- 
> 
> / Raimo Niskanen, Erlang/OTP, Ericsson AB
> _______________________________________________
> erlang-patches mailing list
> erlang-patches@REDACTED
> http://www.erlang.org/mailman/listinfo/erlang-patches

-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
-------------- next part --------------
A non-text attachment was scrubbed...
Name: io_lib_fread.patch
Type: text/x-patch
Size: 4718 bytes
Desc: not available
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20090210/d123e400/attachment.bin>