And another old bug not fixed .... readAppendStream

[marten]

I've mentioned on 07.11.2005 a bug in ibm.software.vasmalltalk, that the whole functionality of the readappend stream is broken.

John O'Keefe posted, that this is a bug and made an internal defect (15180) - but up to now nothing had happened.

I just tested the small example under 7.0.1 and nothing had changed - another hint, that a public bug repository would be really nice.


Marten

[solveig]

Marten,

We have internally implemented a solution, but some of the details are yet to be ironed out. We want to ensure this complies with industry standards.

The Instantiations case number is 1963. It is scheduled for release in 7.5.1.

Regards,
Solveig

[wembley]

Marten -

We've had some discussions in the past as to how an appendable stream should behave when read and write operations are intermixed. Here is what IEEE Std 1003.1, 2004 Edition (C Library Standard) says when describing the fopen() function:

Opening a file with append mode (a as the first character in the mode argument) shall cause all subsequent writes to the file to be forced to the then current end-of-file, regardless of intervening calls to fseek().

When a file is opened with update mode ( '+' as the second or third character in the mode argument), both input and output may be performed on the associated stream. However, the application shall ensure that output is not directly followed by input without an intervening call to fflush() or to a file positioning function ( fseek(), fsetpos(), or rewind()), and input is not directly followed by output without an intervening call to a file positioning function, unless the input operation encounters end-of-file.

Based on this description, I have made the following changes to CfsStream (blue code added):

!CfsFileStream privateMethods !

next: anInteger replaceWith: anObject
"Private - replace the next anInteger elements with: anObject"

| gap charObj newBuff diff |

anInteger <= 0
ifTrue: [^ self].

(((self fileDescriptor oflag bitAnd: OAPPEND) = OAPPEND) and: [self atEnd not])
ifTrue: [self setToEnd].

gap := bufferSize - position.
charObj := anObject isCharacter
ifTrue: [anObject]
ifFalse: [anObject asCharacter].
anInteger > gap
ifTrue: [(diff := anInteger - gap) > bufferSize
ifTrue: ["for effiency: if anInteger is too large, then write the existing buffer
and directly the new buffer"
self writeBuffer.
(newBuff := String new: anInteger) atAllPut: charObj.

self checkError: (fileDescriptor lseek: self position whence: SEEKSET).
self checkError: (fileDescriptor write: newBuff startingAt: 1 nbyte: anInteger).
self filePosition: self position + anInteger.
writeOn :=false]
ifFalse: [
collection replaceFrom: position + 1 to: bufferSize withObject: charObj.
bytes := position := bufferSize.
writeOn := true.
self writeBuffer.
self filePosition: filePosition + bufferSize.

collection replaceFrom: 1 to: diff withObject: charObj.
(position := diff) > bytes ifTrue: [bytes := position].
writeOn := true]]
ifFalse: [
collection replaceFrom: position + 1 to: position + anInteger withObject: charObj.
(position := position + anInteger) > bytes
ifTrue: [bytes := position].
writeOn := true].
!

nextReplace: aSequentialCollection
"Private - replace aSequentialCollection"

| gap siz |

(siz := aSequentialCollection size) = 0
ifTrue: [^ self].

(((self fileDescriptor oflag bitAnd: OAPPEND) = OAPPEND) and: [self atEnd not])
ifTrue: [self setToEnd].

gap := bufferSize - position.
siz > gap
ifTrue: [(siz - gap) > bufferSize
ifTrue: ["for effiency: if aSequentialCollection is too large, then write the existing buffer
and directly write aSequentialCollection to the file"
self writeBuffer.
self checkError: (fileDescriptor lseek: self position whence: SEEKSET).
self checkError: (fileDescriptor write: aSequentialCollection startingAt: 1 nbyte: siz).
self filePosition: self position + siz.
writeOn :=false]
ifFalse: [
collection replaceFrom: position + 1 to: bufferSize with: aSequentialCollection startingAt: 1.
bytes := position := bufferSize.
writeOn := true.
self writeBuffer.
self filePosition: filePosition + bufferSize.
collection replaceFrom: 1 to: siz - gap with: aSequentialCollection startingAt: gap + 1.
(position := siz - gap) > bytes ifTrue: [bytes := position].
writeOn := true]]
ifFalse: [collection replaceFrom: position + 1 to: position + siz with: aSequentialCollection startingAt: 1.
(position := position + siz) > bytes
ifTrue: [bytes := position].
writeOn := true].
!

privateNextPut: aByteOrCharacter
"private - it can be called only if the receiver stream is writable.
Write a byte or aCharacter to the receiver stream."


(((self fileDescriptor oflag bitAnd: OAPPEND) = OAPPEND) and: [self atEnd not])
ifTrue: [self setToEnd].

(position = bufferSize)
ifTrue: [self writeBuffer.
self filePosition: filePosition + self bufferSize].

position < bytes
ifFalse: [bytes := bytes + 1].

writeOn := true.
position := position + 1.

aByteOrCharacter isCharacter
ifTrue: [collection at: position put: aByteOrCharacter]
ifFalse: [collection at: position put: aByteOrCharacter asCharacter].

^aByteOrCharacter! !

These changes align the behavior of CfsStream with the behavior of the CfsFileDescriptor that it wraps. The implication of IEEE 1003.1 and the VA Smalltalk implementation is that the following code will fail in the blue line due to trying to read past EOF:

| aStream result |

aStream := (CfsPath named: 'test3.log') readAppendStreamBinary: false.

1 to: 100 do: [:index |
aStream cr ; nextPutAll: index printString.
index < 100 ifTrue: [aStream reset]].
result := aStream next. "Try to read a character"
aStream close

I consider this to be correct behavior.

[marten]

Hmm, the code

Code: Select all

| aStream result |

aStream := (Filename named: 'test3.log') newReadAppendStream.

1 to: 100 do: [:index |
aStream cr ; nextPutAll: index printString.
index < 100 ifTrue: [aStream reset]].
result := aStream next. "Try to read a character"
aStream close


works under VisualWorks and that's the behaviour I would expect.

For me this is typically a log file. The application used the write stream to write at the at the end of the file and all positioning works (or the normal stream behaviour) on the read stream.

Or to be more clear: and readAppend stream has two positions. The write position is always at the end. The read position is the 'typical' position of that stream - because its the position the programmer can manipulate.

In your example the read position starts at position 0 - and therefore "aStream next" should return the first character in this file. "aStream reset" works on the read part of that stream and not on the write part of that stream.

And your C comment would fit into this idea !?

[wembley]

Marten -

The implementation of file streams in VA Smalltalk is intended to conform to the POSIX.1 standard. I quoted that standard in my previous post to show that the VA Smalltalk implementation (with the changes indicated) matches the standard.

I agree that the VA Smalltalk implementation does not produce the same results as similar VW code. I think this is a separate issue. It is interesting to note that the ANSI Smalltalk group explicitly decided not to attempt standardizing a ReadWriteFileStream -- they only did a ReadFileStream and a WriteFileStream. There is nothing in the rationale for the standard that addresses why they deferred adding ReadWriteFileStream beyond:

There is specification only for the creation and use of readable and writeable file streams. There is not support for read/write
file streams. Nor is there any specification of file or directory manipulation, as these facilities are considered by the
Committee to be too platform-dependent and too implementation-dependent to standardize at this time, and it is felt that
streaming is adequate.

In summary, I view your original report on the VisualAge Smalltalk newsgroup that writing to an appendable file gave incorrect results to be a defect that is corrected by the code change I posted.

[wembley]

Marten -

I neglected to address one of your questions:

In your example the read position starts at position 0 - and therefore "aStream next" should return the first character in this file. "aStream reset" works on the read part of that stream and not on the write part of that stream.

And your C comment would fit into this idea !?

Actually, I purposely omitted the reset for the last iteration of the loop so that the file position would be at the end of the file when the next message was sent. If I had not done that then your observation that next would return the first character in the stream would be correct.

[marten]

Actually, I purposely omitted the reset for the last iteration of the loop so that the file position would be at the end of the file when the next message was sent. If I had not done that then your observation that next would return the first character in the stream would be correct.

I think, that both VW and VA implementation fit the IEEE Std 1003.1, 2004 Edition (C Library Standard) as you mentioned it. My original posting is simply mentioning a bug - and you seem to have corrected it.

The other thing is the semantic programming model and VW has two file positions, which are handled independently. The programmer has no control about the write file position, but can only controls the read file position via his/her stream commands.

VA has only one position and this make the whole stuff very complicated to use. Each programmer (using this stream) must maintain its own read-position - external to this stream, because a write call (perhaps by a log manager in another module) may change the position .... one can see this in your own example.

But ok, leave it as it is. Nobody seems to use this class anyway in read AND append mode at the same time and everyone can write its own subclass to get the (in my opinion) much clearer VW behaviour ... and
I like to have the same behaviour at that level between VW and VA.

[wembley]

Marten -

Please don't misunderstand me. I agree that the notion of maintaining separate read and write pointers could be useful. But I want to separate that discussion from the problem of writing corrupt files (whether read/write or write-only) when append is specified. Corrupt files is a bug, separate read and write pointers is an enhancement

I'm going to move on with the fix.