Bug in FileSystem
Locked
 
|
FSReadStream >> next: count
| result | result := ByteArray new: count. handle at: position read: result startingAt: 1 count: count. position := position + 1. ^ result shouldn't be FSReadStream >> next: count | result | result := ByteArray new: count. handle at: position read: result startingAt: 1 count: count. position := position + count. ^ result At least with that (among some extensions) Fuel tests pass with FS :) Cheers -- Mariano http://marianopeck.wordpress.com |
|
|
sounds like :)
Stef On Jan 7, 2012, at 1:10 PM, Mariano Martinez Peck wrote: > FSReadStream >> next: count > | result | > result := ByteArray new: count. > handle at: position read: result startingAt: 1 count: count. > position := position + 1. > ^ result > > > shouldn't be > > FSReadStream >> next: count > | result | > result := ByteArray new: count. > handle at: position read: result startingAt: 1 count: count. > position := position + count. > ^ result > > At least with that (among some extensions) Fuel tests pass with FS :) > > Cheers > > -- > Mariano > http://marianopeck.wordpress.com > |
|
|
On 07.01.2012 13:20, Stéphane Ducasse wrote:
> sounds like :) > > Stef > > On Jan 7, 2012, at 1:10 PM, Mariano Martinez Peck wrote: > >> FSReadStream>> next: count >> | result | >> result := ByteArray new: count. >> handle at: position read: result startingAt: 1 count: count. >> position := position + 1. >> ^ result >> >> >> shouldn't be >> >> FSReadStream>> next: count >> | result | >> result := ByteArray new: count. >> handle at: position read: result startingAt: 1 count: count. >> position := position + count. >> ^ result >> >> At least with that (among some extensions) Fuel tests pass with FS :) >> >> Cheers >> >> -- >> Mariano >> http://marianopeck.wordpress.com >> > It doesn't handle the case where you try to read past end of stream. next: count ^self nextInto: (ByteArray new: count) would probably be better. Cheers, Henry |
|
|
In reply to this post by Stéphane Ducasse
On 07.01.2012 14:05, Henrik Sperre Johansen wrote:
> On 07.01.2012 13:20, Stéphane Ducasse wrote: >> sounds like :) >> >> Stef >> >> On Jan 7, 2012, at 1:10 PM, Mariano Martinez Peck wrote: >> >>> FSReadStream>> next: count >>> | result | >>> result := ByteArray new: count. >>> handle at: position read: result startingAt: 1 count: count. >>> position := position + 1. >>> ^ result >>> >>> >>> shouldn't be >>> >>> FSReadStream>> next: count >>> | result | >>> result := ByteArray new: count. >>> handle at: position read: result startingAt: 1 count: count. >>> position := position + count. >>> ^ result >>> >>> At least with that (among some extensions) Fuel tests pass with FS :) >>> >>> Cheers >>> >>> -- >>> Mariano >>> http://marianopeck.wordpress.com >>> >> > Still wrong though, I think... > It doesn't handle the case where you try to read past end of stream. > > next: count > ^self nextInto: (ByteArray new: count) > > would probably be better. > > Cheers, > Henry > > 1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations. 2) FSHandle >>at: offset read: buffer startingAt: start count: count is a weirdly named selector for what it does imho... Doesn't #at: offset read: count into: buffer startingAt: start make more sense? Cheers, Henry |
|
|
2012/1/7 Henrik Sperre Johansen <[hidden email]>:
> On 07.01.2012 14:05, Henrik Sperre Johansen wrote: >> >> On 07.01.2012 13:20, Stéphane Ducasse wrote: >>> >>> sounds like :) >>> >>> Stef >>> >>> On Jan 7, 2012, at 1:10 PM, Mariano Martinez Peck wrote: >>> >>>> FSReadStream>> next: count >>>> | result | >>>> result := ByteArray new: count. >>>> handle at: position read: result startingAt: 1 count: count. >>>> position := position + 1. >>>> ^ result >>>> >>>> >>>> shouldn't be >>>> >>>> FSReadStream>> next: count >>>> | result | >>>> result := ByteArray new: count. >>>> handle at: position read: result startingAt: 1 count: count. >>>> position := position + count. >>>> ^ result >>>> >>>> At least with that (among some extensions) Fuel tests pass with FS :) >>>> >>>> Cheers >>>> >>>> -- >>>> Mariano >>>> http://marianopeck.wordpress.com >>>> >>> >> Still wrong though, I think... >> It doesn't handle the case where you try to read past end of stream. >> >> next: count >> ^self nextInto: (ByteArray new: count) >> >> would probably be better. >> >> Cheers, >> Henry >> >> > As for the API: > > 1) There's no next: into: (which returns the amount read). It's really the > only way to do buffers without garbage allocations. > 2) FSHandle >>at: offset read: buffer startingAt: start count: count is a > weirdly named selector for what it does imho... > Doesn't #at: offset read: count into: buffer startingAt: start make more > sense? > Sure. But without reading source code I also wonder why mixing two operations into a single ? I mean seek() and read() Since the underlying OS handle is most probably a stream, it should preserve its own position. Is it because the handle might be shared and FS thus has to provide some kind of atomic operation ? Nicolas > Cheers, > Henry > |
|
|
In reply to this post by Henrik Sperre Johansen
On 07 Jan 2012, at 14:19, Henrik Sperre Johansen wrote: > 1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations. The 2 key methods for efficient IO are: readInto: collection startingAt: offset count: requestedCount "Read requestedCount elements into collection starting at offset, returning the number of elements read, there could be less elements available." next: count putAll: collection startingAt: offset "Write count bytes from collection starting at offset." All other bulk operations can be written in terms of these (see for example Zodiac streams). Sven |
|
|
2012/1/7 Sven Van Caekenberghe <[hidden email]>:
> > On 07 Jan 2012, at 14:19, Henrik Sperre Johansen wrote: > >> 1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations. > > The 2 key methods for efficient IO are: > > readInto: collection startingAt: offset count: requestedCount > "Read requestedCount elements into collection starting at offset, > returning the number of elements read, there could be less elements available." > > next: count putAll: collection startingAt: offset > "Write count bytes from collection starting at offset." > > All other bulk operations can be written in terms of these (see for example Zodiac streams). > > Sven +1. All streams should provide these API, and if possible, provide an optimized implementation. It was not the case in Squeak, i started to refactor a bit, but incrementally modifying this dinosaurus without creating circularities is a sport! I still have 3 years old refactorings unpublished because I'm not sure how to reproduce the changes in a safe order ;) Nicolas |
|
|
How can we build and improve FS? I'm too busy with other topics but I would love to get some momentum on that topic. Stef > On 07 Jan 2012, at 14:19, Henrik Sperre Johansen wrote: >> >>> 1) There's no next: into: (which returns the amount read). It's really the only way to do buffers without garbage allocations. >> >> The 2 key methods for efficient IO are: >> >> readInto: collection startingAt: offset count: requestedCount >> "Read requestedCount elements into collection starting at offset, >> returning the number of elements read, there could be less elements available." >> >> next: count putAll: collection startingAt: offset >> "Write count bytes from collection starting at offset." >> >> All other bulk operations can be written in terms of these (see for example Zodiac streams). >> >> Sven > > +1. > All streams should provide these API, and if possible, provide an > optimized implementation. > It was not the case in Squeak, i started to refactor a bit, but > incrementally modifying this dinosaurus without creating circularities > is a sport! > I still have 3 years old refactorings unpublished because I'm not sure > how to reproduce the changes in a safe order ;) > > Nicolas > |
|
|
In reply to this post by Henrik Sperre Johansen
On Sat, Jan 7, 2012 at 2:05 PM, Henrik Sperre Johansen <[hidden email]> wrote:
Indeed. Cheers, -- Mariano http://marianopeck.wordpress.com |
|
|
In reply to this post by Henrik Sperre Johansen
As for the API: Hi Henry. One of the extensions I needed to add to FSReadStream in order to make it work with Fuel was exatly #next:into: The method is: next: number into: aCollection | count | count := handle at: position read: aCollection startingAt: 1 count: number. position := position + count. ^ count < aCollection size ifTrue: [aCollection first: count] ifFalse: [aCollection] I didn't understand what you meant by "It's really the only way to do buffers without garbage allocations" So...should I integrate this method #next:into: as it is together to the fix to #read: 2) FSHandle >>at: offset read: buffer startingAt: start count: count is a weirdly named selector for what it does imho... -- Mariano http://marianopeck.wordpress.com |
|
|
So, here it is: http://code.google.com/p/pharo/issues/detail?id=5161
On Sun, Jan 8, 2012 at 2:39 PM, Mariano Martinez Peck <[hidden email]> wrote:
-- Mariano http://marianopeck.wordpress.com |
|
|
In reply to this post by Mariano Martinez Peck
But what if count makes handle pass over the end of the file?
-- Cesar Rabak Em 07/01/2012 10:10, Mariano Martinez Peck < [hidden email] > escreveu: FSReadStream >> next: count | result | result := ByteArray new: count. handle at: position read: result startingAt: 1 count: count. position := position + 1. ^ result shouldn't be FSReadStream >> next: count | result | result := ByteArray new: count. handle at: position read: result startingAt: 1 count: count. position := position + count. ^ result At least with that (among some extensions) Fuel tests pass with FS :) Cheers -- Mariano http://marianopeck.wordpress.com |
«
Return to Pharo Smalltalk Developers
|
1 view|%1 views
| Free forum by Nabble | Edit this page |