LibTiff Mailing List
TIFF and LibTiff Mailing List Archive
February 2010
Previous Thread
Next Thread
Previous by Thread
Next by Thread
Previous by Date
Next by Date
Contact
The TIFF Mailing List Homepage
This list is run by Frank Warmerdam
Archive maintained by AWare Systems
Thread
2010.03.02 08:14 "Re: 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). Bob Friesenhahn wrote: > 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) { - size_t size_io = (size_t) size; - if ((tmsize_t) size_io != size) - { - errno=EINVAL; - return (tmsize_t) -1; - } - return ((tmsize_t) read((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 = 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