search

pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.19k stars 351 forks source link

ZnUTF8Encoder>>#ensureAtBeginOfCodePointOnStream: sends bitAnd: to nil when setting stream position to end #14455

Open j-brant opened 1 year ago

j-brant commented 1 year ago

Bug description Setting FileReference readStream position to the end causes DNU error for bitAnd: sent to nil.

To Reproduce Evaluate:

| file stream |
file := FileSystem memory / 'test'.
file writeStreamDo: [ :s | s nextPut: $a ].
stream := file readStream.
stream position: file size.

Expected behavior The stream should be positioned at the end so atEnd returns true.

Version information: Pharo-11.0.0+build.702.sha.a844e9ab79ba34a9f3999f01c81a405408c4dc55 (64 Bit)

Expected development cost Adding an atEnd check to the ZnUTF8Encoder>>#ensureAtBeginOfCodePointOnStream: method fixes the issue. 

    [ stream atEnd not and: [ (stream peek bitAnd: 2r11000000) == 2r10000000 ] ] whileTrue: [ stream back ]
svenvc commented 7 months ago

This was already fixed in https://github.com/svenvc/zinc/commit/6882f5f06096d56ddee79ff278ecbd09248e7708

svenvc commented 7 months ago

this can be closed as duplicate & solved

j-brant commented 2 months ago

It appears that the fix from Zinc was never included in Pharo 11-13.

Ducasse commented 2 months ago

So @svenvc can you let us know what we should do?

svenvc commented 2 months ago

take over the fix from @daniels220 I guess. it is only a very small change, an edge case guard.

Ducasse commented 2 months ago

I do not get it should we use a different version of Zinc? I have no idea what is @daniels220' fix?

daniels220 commented 1 month ago

@Ducasse the fix is the change in https://github.com/svenvc/zinc/commit/6882f5f06096d56ddee79ff278ecbd09248e7708. But for Pharo I would think the appropriate change would be to ensure the version of Zinc that ships with Pharo is up-to-date with the latest (or latest stable tag or somesuch if that exists) in the Zinc repo, which would include the fix.