The changes in SqueakSSL-Core-eem.32 and in Network-eem.186 are, I
think, morally equivalent to the changes in Network-tonyg.186. Levente's
suggestion in response to that was that the change would be better in
#indexOfSubCollection:startingAt:ifAbsent. I liked that idea, and
implemented it in Collections-tonyg.733.1 (sorry again for that branch)
and CollectionsTests-tonyg.275.

Should the fix be in *both* places? Just one? If the latter, which?

My opinion is that

 - the -eem changes are fine, and don't need reverting. Similarly,
   the Network-tonyg.186 commit is obsolete and I'd retract it from
   the inbox if I could

 - the Collections-tonyg.733.1 change suggested by Levente is a good
   idea, and should be merged into the trunk (with the corresponding
   tests)

I'm still confused about exactly what I should have done to avoid that
.733.1 branch, given that a -tonyg.734 existed and was unrelated and
needed separate review. (It still does btw! It fixes a mantis issue fwiw)

Any clarifications on this aspect of the development process would be
very welcome!

Thanks,
  Tony



On 02/25/2017 06:46 PM, commits at source.squeak.org wrote:
> Eliot Miranda uploaded a new version of SqueakSSL-Core to project The Trunk:
> http://source.squeak.org/trunk/SqueakSSL-Core-eem.32.mcz
> 
> ==================== Summary ====================
> 
> Name: SqueakSSL-Core-eem.32
> Author: eem
> Time: 25 February 2017, 10:46:29.171578 am
> UUID: d57e1fe9-c199-47bf-81c7-abe7964f34f9
> Ancestors: SqueakSSL-Core-ul.31
> 
> Fix slip in upToAll:limit: that causes errors in the SocketStreamTests>>testUpToAllCrlf* tests.  See Network-eem.186.
> 
> =============== Diff against SqueakSSL-Core-ul.31 ===============
> 
> Item was changed:
>   ----- Method: SecureSocketStream>>upToAll:limit: (in category 'private-compat') -----
>   upToAll: aStringOrByteArray limit: nBytes
>   	"Pre Squeak 4.2 compatibility"
>   
>   	| index sz result searchedSoFar target |
>   	"Deal with ascii vs. binary"
> + 	target := self isBinary
> + 				ifTrue:[aStringOrByteArray asByteArray]
> + 				ifFalse:[aStringOrByteArray asString].
> - 	self isBinary
> - 		ifTrue:[target := aStringOrByteArray asByteArray]
> - 		ifFalse:[target := aStringOrByteArray asString].
>   
>   	sz := target size.
>   	"Look in the current inBuffer first"
>   	index := inBuffer indexOfSubCollection: target
> + 						startingAt: (lastRead - sz + 2 max: 1).
> - 						startingAt: lastRead - sz + 2.
>   	(index > 0 and: [(index + sz) <= inNextToWrite]) ifTrue: ["found it"
>   		result := self nextInBuffer: index - lastRead - 1.
>   		self skip: sz.
>   		^ result
>   	].
>   
>   	[searchedSoFar :=  self inBufferSize.
>   	"Receive more data"
>   	self receiveData.
>   	recentlyRead > 0] whileTrue:[
>   
>   		"Data begins at lastRead + 1, we add searchedSoFar as offset and 
>   		backs up sz - 1 so that we can catch any borderline hits."
>   
>   		index := inBuffer indexOfSubCollection: target
>   						startingAt: (lastRead + searchedSoFar - sz + 2 max: 1).
>   		(index > 0 and: [(index + sz) <= inNextToWrite]) ifTrue: ["found it"
>   			result := self nextInBuffer: index - lastRead - 1.
>   			self skip: sz.
>   			^ result
>   		].
>   		"Check if we've exceeded the max. amount"
>   		(nBytes notNil and:[inNextToWrite - lastRead > nBytes]) 
>   			ifTrue:[^self nextAllInBuffer].
>   	].
>   
>   	"not found and (non-signaling) connection was closed"
>   	^self nextAllInBuffer!
> 
>