View Issue Details

IDProjectCategoryView StatusLast Update
0036758FPCPackagespublic2020-03-07 19:07
ReporterMarcin Wiazowski Assigned ToMichael Van Canneyt  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0036758: Small improvement for Tcompressionstream
DescriptionWhen writing compressed data to a stream, the Tcompressionstream implementation uses either stream's Write or WriteBuffer method.

Using the WriteBuffer method is a good way of writing data - in case of writing error, an exception is raised.

On the contrary - in case of writing error - the Write method just returns smaller number of written bytes as a result. The problem is that silent data loss will occur in such case:



Function TCompressionstream.ClearOutBuffer : Integer;
begin
  { Flush the buffer to the stream and update progress }
  Result:=source.write(Fbuffer^,bufsize); <====== returned Result may be less than bufsize - and no exception is raised
  inc(compressed_written,Result);

  { reset output buffer } <====== data buffer is cleared, although not fully written to the output stream
  Fstream.next_out:=Fbuffer;
  Fstream.avail_out:=bufsize;
end;



To avoid silent data loss, the attached patch converts Write call to a WriteBuffer call.

In addition, due to requesting "bufsize-Fstream.avail_out" bytes to be written, instead of just "bufsize", the modified ClearOutBuffer implementation may be also used to replace a bunch of code in a Tcompressionstream.flush implementation.

Since both these changes affect the same code, both of them are contained in the attached patch.
TagsNo tags attached.
Fixed in Revision44282
FPCOldBugId
FPCTarget4.0.0
Attached Files

Activities

Marcin Wiazowski

2020-03-04 23:49

reporter  

patch.diff (1,330 bytes)   
Index: zstream.pp
===================================================================
--- zstream.pp	(revision 44263)
+++ zstream.pp	(working copy)
@@ -56,7 +56,7 @@
 
         Tcompressionstream=class(Tcustomzlibstream)
         private
-          function ClearOutBuffer: Integer;
+          procedure ClearOutBuffer;
         protected
           raw_written,compressed_written: int64;
         public
@@ -206,13 +206,12 @@
   get_compressionrate:=100*compressed_written/raw_written;
 end;
 
-Function TCompressionstream.ClearOutBuffer : Integer;
+procedure TCompressionstream.ClearOutBuffer;
 
-
 begin
   { Flush the buffer to the stream and update progress }
-  Result:=source.write(Fbuffer^,bufsize);
-  inc(compressed_written,Result);
+  source.writebuffer(Fbuffer^,bufsize-Fstream.avail_out);
+  inc(compressed_written,bufsize-Fstream.avail_out);
   progress(self);
   { reset output buffer }
   Fstream.next_out:=Fbuffer;
@@ -235,13 +234,7 @@
       raise Ecompressionerror.create(zerror(err));
   until false;
   if Fstream.avail_out<bufsize then
-    begin
-      source.writebuffer(FBuffer^,bufsize-Fstream.avail_out);
-      inc(compressed_written,bufsize-Fstream.avail_out);
-      progress(self);
-      Fstream.next_out:=Fbuffer;
-      Fstream.avail_out:=bufsize;
-    end;
+    ClearOutBuffer;
 end;
 
 
patch.diff (1,330 bytes)   

Michael Van Canneyt

2020-03-07 16:01

administrator   ~0121434

Checked and applied, thank you very much !

Marcin Wiazowski

2020-03-07 19:07

reporter   ~0121440

Thanks!

Issue History

Date Modified Username Field Change
2020-03-04 23:49 Marcin Wiazowski New Issue
2020-03-04 23:49 Marcin Wiazowski File Added: patch.diff
2020-03-07 16:01 Michael Van Canneyt Assigned To => Michael Van Canneyt
2020-03-07 16:01 Michael Van Canneyt Status new => resolved
2020-03-07 16:01 Michael Van Canneyt Resolution open => fixed
2020-03-07 16:01 Michael Van Canneyt Fixed in Version => 3.3.1
2020-03-07 16:01 Michael Van Canneyt Fixed in Revision => 44282
2020-03-07 16:01 Michael Van Canneyt FPCTarget => 4.0.0
2020-03-07 16:01 Michael Van Canneyt Note Added: 0121434
2020-03-07 19:07 Marcin Wiazowski Status resolved => closed
2020-03-07 19:07 Marcin Wiazowski Note Added: 0121440