Re: [Help-smalltalk] [PATCH 1/2] streams: Fix the ConcatenatedStream>>#copyFrom:to: implementation

[Paolo Bonzini]

Il 06/10/2012 18:32, Holger Hans Peter Freyther ha scritto:
> From: Holger Hans Peter Freyther <address@hidden>
> 
> $ gst-convert -F squeak -o out.st File1.st File2.st File3.st
> 
> failed with nil doesn't understand copyFrom: to:. This happens when
> the stream is at the end and 'self stream' returns nil and the last
> stream is not migrated to 'last'. Check for this condition in the
> copyFrom:to:.
> 
> 2012-10-06  Holger Freyther  <address@hidden>
> 
>       * kernel/StreamOps.st: Modify ConcatenatedStream>>#copyFrom:to:
>       when at the end of the last stream.
>       * tests/stream.st: Add testcase for ConcatenatedStream>>#copyFrom:to:
>       at the end of the last stream.
>       * tests/stream.ok: Create the test result.
>       * tests/testsuite.at: Add the stream.st to the regression tests.
> ---
>  ChangeLog           |    9 +++++++++
>  kernel/StreamOps.st |    2 +-
>  tests/testsuite.at  |    1 +
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 91c384e..9ddd89e 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,12 @@
> +2012-10-06  Holger Freyther  <address@hidden>
> +
> +     * kernel/StreamOps.st: Modify ConcatenatedStream>>#copyFrom:to:
> +     when at the end of the last stream.
> +     * tests/stream.st: Add testcase for ConcatenatedStream>>#copyFrom:to:
> +     at the end of the last stream.
> +     * tests/stream.ok: Create the test result.
> +     * tests/testsuite.at: Add the stream.st to the regression tests.
> +
>  2012-10-02  Holger Freyther  <address@hidden>
>  
>       * tests/stcompiler.st: Add testcase for pragma parsing.
> diff --git a/kernel/StreamOps.st b/kernel/StreamOps.st
> index 9fd1769..0525e5f 100644
> --- a/kernel/StreamOps.st
> +++ b/kernel/StreamOps.st
> @@ -158,7 +158,7 @@ Stream subclass: ConcatenatedStream [
>  
>       <category: 'all'>
>       | adjust stream |
> -     stream := self stream.
> +     stream := self stream ifNil: [streams first].
>       end + 1 = start ifTrue: [^''].
>       adjust := end <= startPos 
>           ifTrue: [stream := last. lastStart]
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index ffa3919..d64f061 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -52,6 +52,7 @@ AT_DIFF_TEST([getopt.st])
>  AT_DIFF_TEST([quit.st])
>  AT_DIFF_TEST([pools.st])
>  AT_DIFF_TEST([shape.st])
> +AT_DIFF_TEST([stream.st])
>  
>  AT_BANNER([Other simple tests.])
>  AT_DIFF_TEST([ackermann.st])
> 

You didn't include stream.st and stream.ok. :)

Does this work instead?  As you mentioned the problem is that
last was not initialized.

diff --git a/kernel/StreamOps.st b/kernel/StreamOps.st
index 9fd1769..63bea8a 100644
--- a/kernel/StreamOps.st
+++ b/kernel/StreamOps.st
@@ -78,11 +78,13 @@ Stream subclass: ConcatenatedStream [
     stream [
        <category: 'all'>
        | s |
-       [(s := streams first) atEnd] whileTrue: 
-               [streams size = 1 ifTrue: [^nil].
-               lastStart := startPos.
+        "This is somewhat performance-sensitive, so avoid testing for an
+         empty collection."
+       [(s := streams at: 1) atEnd] whileTrue: 
+               [lastStart := startPos.
                startPos := startPos + curPos.
                curPos := 0.
+                streams size = 1 ifTrue: [last := streams first. ^nil].
                last := streams removeFirst].
        ^s
     ]

Paolo