Returning EIO (a small positive integer) as the result of 
archive_tar_file::read()
on an error isn't a good idea, it's indistinguishable from successfully reading
a small number of bytes.

This causes io_stream::copy() to spin forever if it's reading from an archive 
stream
which terminates unexpectedly early, which can happen on an decompression error.

So, return -1 and set lasterr instead.  This matches the behaviour of POSIX
read(), so shouldn't surprise anyone :-)

Make the same change for archive_tar_file::write(), archive_tar_file::tell()
and archive_tar_file::peek() for consistency

2011-04-08  Jon TURNEY  <[email protected]>

        * archive_tar_file.cc (read, write, peek, seek): Consistently return -1
        and set lasterr on an error.

Signed-off-by: Jon TURNEY <[email protected]>
---
 archive_tar_file.cc |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/archive_tar_file.cc b/archive_tar_file.cc
index 29a2df7..5ffb3a1 100644
--- a/archive_tar_file.cc
+++ b/archive_tar_file.cc
@@ -65,7 +65,7 @@ ssize_t archive_tar_file::read (void *buffer, size_t len)
          /* unexpected EOF or read error in the tar parent stream */
          /* the user can query the parent for the error */
          state.lasterr = EIO;
-         return EIO;
+         return -1;
        }
     }
   return 0;
@@ -75,7 +75,8 @@ ssize_t archive_tar_file::read (void *buffer, size_t len)
 ssize_t archive_tar_file::write (const void *buffer, size_t len)
 {
   /* write not supported */
-  return EBADF;
+  state.lasterr = EBADF;
+  return -1;
 }
 
 /* read data without removing it from the class's internal buffer */
@@ -96,7 +97,7 @@ ssize_t archive_tar_file::peek (void *buffer, size_t len)
          /* unexpected EOF or read error in the tar parent stream */
          /* the user can query the parent for the error */
          state.lasterr = EIO;
-         return EIO;
+         return -1;
        }
     }
   return 0;
@@ -113,6 +114,7 @@ archive_tar_file::seek (long where, io_stream_seek_t whence)
 {
   /* nothing needs seeking here yet. Implement when needed 
    */
+  state.lasterr = EINVAL;
   return -1;
 }
 
-- 
1.7.4

Reply via email to