Forum > FPC development

Bug in TReadBufStream and proposed fix

(1/10) > >>

glorfin:
I found that TReadBufStream.Seek function did not work properly, especially when seeking backwards.
Indeed, here is its code:

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function TReadBufStream.Seek(const Offset: Int64; Origin: TSeekOrigin): Int64;begin  FakeSeekForward(Offset,Origin,FTotalPos);  Result:=FTotalPos; // Pos updated by fake readend; First, and important. procedure TStream.FakeSeekForward does not seek backward (as the name suggests).

Second, updated after connet of howardpc:
Third parameter in FakeSeekForward has const modifier, so it should not be changed by the call.
Then what means comment "Position updated by fake read"?

My suggestion is to rewrite Seek function as follows (in my project this worked nicely):


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function TReadBufStream.Seek(const Offset: Int64; Origin: TSeekOrigin): Int64;var  NewTotalPos, Delta, NewBufPos: Int64;begin  case Origin of    soCurrent:      NewTotalPos := FTotalPos + Offset;    soBeginning: begin      if Offset < 0 then        InvalidSeek;      NewTotalPos := Offset;    end;    soEnd: begin      if Offset > 0 then        InvalidSeek;      NewTotalPos := Size + Offset;    end;  end;  if NewTotalPos > Size then    NewTotalPos := Size;  if NewTotalPos < 0 then    NewTotalPos := 0;  Delta := NewTotalPos - FTotalPos;  NewBufPos := Delta + FBufPos;  if (NewBufPos > 0) and (NewBufPos < FBufSize) then  begin    FBufPos := NewBufPos;  //if Seek function moves inside the data which are in the Buffer, we only reposition FBufPos    FTotalPos := NewTotalPos;  end  else // otherwise find desired place in the source stream and invalidate buffer  begin    FTotalPos := FSource.Seek(Offset,Origin);    FBufPos := 0;    FBufSize := 0;  end;  Result := FTotalPos;end; 
By the way, what this function must return? I, as mention in comment, return new position relative to beginning. Is it expected?

Best regards to all!

howardpc:

--- Quote from: glorfin on August 01, 2017, 04:44:05 pm ---
--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---function TReadBufStream.Seek(const Offset: Int64; Origin: TSeekOrigin): Int64;begin  FakeSeekForward(Offset,Origin,FTotalPos);  Result:=FTotalPos; // Pos updated by fake readend; ... Origin parameter has const modifier, so it should not be changed by the call.

--- End quote ---

I don't know where this code comes from, but the Seek call does not change Origin, it simply plugs its value into the call to Fake. If it did change its value, and it is indeed a const parameter, the compiler would refuse the construction.

glorfin:
Oops, sorry, in original post I meant third parameter, FTotalPos. It is supposed to be changed by the call (see the comment), but in fact it is not.
But this was side note; important is that you could not move backwards with this seek function. That is why I had to rewrite it.

Remy Lebeau:

--- Quote from: glorfin on August 01, 2017, 04:44:05 pm ---By the way, what this function must return? I, as mention in comment, return new position relative to beginning. Is it expected?

--- End quote ---

Yes, Seek() is meant to return the new absolute position from the beginning of the stream.

glorfin:

--- Quote from: Remy Lebeau on August 02, 2017, 01:40:37 am ---Yes, Seek() is meant to return the new absolute position from the beginning of the stream.

--- End quote ---
Thank you!
Now would be nice to incorporate this new code into BufStream, because old one really does not work.

Navigation

[0] Message Index

[#] Next page

Go to full version