2010.02.26 08:11 "[Tiff] Patch for tif_unix.c", by Eric Doenges

2010.03.02 08:14 "Re: [Tiff] Patch for tif_unix.c", by Eric Doenges

Hi all,

thanks for the feedback. I've reworked the patch to declare 'bytes' as a ssize_t instead of size_t (this is actually quite an embarrassing error, as the compiler helpfully warns that it's not going to work as a size_t).

There is some code which looks very much like this in GraphicsMagick

but with the primary intention to deal with system calls which are

interrupted by a signal. While the approach used in GraphicsMagick

would deal with the short read/write issue, I had never considered

that a system would have an issue with the size of the data passed.

Writing truely portable I/O code is a source of endless joy =8^(

The similar code in GraphicsMagick is in the FilePositionRead() and

FilePositionWrite() functions viewable on the web at "http://cvs.graphicsmagick.org/cgi-bin/cvsweb.cgi/GraphicsMagick/magick/pixel_cache.c?rev=1.67;content-type=text%2Fx-cvsweb-markup".

IMO that code isn't right either. Unless you can guarantee that 'length' will always be < SSIZE_MAX, you need to check for it. And if the call is interrupted before any data could be read or written, the function will return with -1 and errno=EINTR and not read or write all data as the comments claim.

Which leads back to two questions about my patch:

  1. Can I simply assume that SSIZE_MAX will be defined in <limits.h>, and that <limits.h> will be available for all platforms that matter to libtiff ?
  2. What is the correct response to receiving EINTR (i.e. being interrupted before any data is read/written) ? Should the IO operation simply be retried in _tiffReadProc/_tiffWriteProc, or should the caller handle it ?

It does not seem like your patch deals with the case where the system

call returns -1 to indicate an error. This is important to deal with
properly since otherwise the software could lock up. Can you fix this
and re-submit your patch? While it is useful to post to this list,
always remember to open a bug report on libtiff Bugzilla and post your
patch there as well.

Unfortunately, Bugzilla isn't working for me. Following the link given on the libtiff homepage (http://bugzilla.remotesensing.org/enter_bug.cgi?product=libtiff) gives me a 404:

--- SNIP ---

The requested URL /enter_bug.cgi was not found on this server.

Additionally, a 404 Not Found error was encountered while trying to use an ErrorDocument to handle the request.

--- SNIP ---

With kind regards,
Eric Dönges
--
Dr. Eric Dönges doenges@mvtec.com

MVTec Software GmbH | Neherstr. 1 | 81675 München  | Germany

www.mvtec.com | Tel: +49 89 457695-0 | Fax: +49 89 457695-55

Geschäftsführer: Dr. Wolfgang Eckstein, Dr. Olaf Munkelt Amtsgericht München HRB 114695

Index: libtiff/tif_unix.c

=================================================================== RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_unix.c,v retrieving revision 1.21

diff -u -r1.21 tif_unix.c

--- libtiff/tif_unix.c  18 Jul 2007 13:46:41 -0000      1.21
+++ libtiff/tif_unix.c  2 Mar 2010 08:00:58 -0000

@@ -39,6 +39,7 @@

 #include <stdarg.h>
 #include <stdlib.h>
+#include <limits.h>
 #include <sys/stat.h>

 #ifdef HAVE_UNISTD_H
@@ -58,25 +59,53 @@
 static tmsize_t
 _tiffReadProc(thandle_t fd, void* buf, tmsize_t size)
 {

+        ssize_t bytes;
+        size_t total, request, size_io = (size_t) size;
+        if ((tmsize_t) size_io != size)
+        {
+                errno=EINVAL;
+                return (tmsize_t) -1;
+        }
+
+        for (total = 0; total < size_io; total += bytes)
+        {
+                request = size_io - total;
+                if (request > SSIZE_MAX)
+                        request = SSIZE_MAX;
+
+                bytes = read((int) fd, (char *) buf + total, request);
+                if (0 > bytes)

+                        return (tmsize_t) -1;
+                else if (0 == bytes)
+                        break;
+        }
+        return (tmsize_t) total;
 }

 static tmsize_t
 _tiffWriteProc(thandle_t fd, void* buf, tmsize_t size)
 {
-       size_t size_io = (size_t) size;
-       if ((tmsize_t) size_io != size)
-       {
-               errno=EINVAL;
-               return (tmsize_t) -1;
-       }
-       return ((tmsize_t) write((int) fd, buf, size_io));

+        ssize_t bytes;
+        size_t total, request, size_io = (size_t) size;
+        if ((tmsize_t) size_io != size)
+        {
+                errno=EINVAL;
+                return (tmsize_t) -1;
+        }
+
+        for (total = 0; total < size_io; total += bytes)
+        {
+                request = size_io - total;
+                if (request > SSIZE_MAX)
+                  request = SSIZE_MAX;
+
+                bytes = write((int) fd, (char *) buf + total, request);
+                if (0 > bytes)
+                        return (tmsize_t)-1;
+                else if (0 == bytes)
+                        break;
+        }
+        return (tmsize_t) total;

 }

 static uint64