Smalltalk › Squeak › Squeak - Dev

The Trunk: SqueakSSL-Core-eem.32.mcz

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

3 messages Options

commits-2

The Trunk: SqueakSSL-Core-eem.32.mcz

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!

Tony Garnock-Jones-3

Fixes for binary-mode #upToAll:, and development process questions (was Re: The Trunk: SqueakSSL-Core-eem.32.mcz)

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, [hidden email] 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!
>
>

Levente Uzonyi

Re: Fixes for binary-mode #upToAll:, and development process questions (was Re: The Trunk: SqueakSSL-Core-eem.32.mcz)

Hi Tony,

First of all, sorry for the delay.

On Sat, 25 Feb 2017, Tony Garnock-Jones wrote:

> 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

I moved it to the Treated Inbox.

>
> - 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 merged it (-ul.735) and pushed it to the Trunk. Hopefully the Trunk
won't blow up. :)

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

You could have saved it as -tonyg.735. The version numbers don't have to
be continuous, it's enough if they are strictly increasing.

The best practice is to always start from some version of Trunk and commit
changes from that point to the Inbox, because such versions can always be
merged.
If you have changes which require multiple versions, then you should
request people not to commit to the Trunk, because if someone does,
then all your versions will have to be merged.

Levente

>
> Thanks,
> Tony
>
>
>
> On 02/25/2017 06:46 PM, [hidden email] 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!
>>
>>