2008.07.28 08:49 "[Tiff] CVS access", by Mateusz Łoskot

2008.08.13 04:23 "[Tiff] tif_win32.c patch proposal (was: windows 64 bit build)", by Edward Lam

OK, second take then. We keep everything the same but fix it differently in tif_win32.c. In TIFFFdOpen(), we use _get_osfhandle() to convert the given ifd into a HANDLE that we pass to TIFFClientOpen(). In the calls to TIFFFdOpen(), we call _open_osfhandle() to allocate an fd corresponding to the HANDLE.

If you can submit a tested patch, I will be happy to commit it.

Ok, here's a patch attached for discussion. The approach I previously mentioned ran into a slight snag. Once you've created an fd for a HANDLE, the C runtime library (CRT) takes ownership of it. Thus, when you call _close(fd), it closes the associated HANDLE as well. Plus, in _tiffCloseProc(), there was no way (that I found) to query the CRT to return the associated fd given the HANDLE. So in the end, I did something more drastic, albeit cleaner.

I changed the calls of TIFFClientOpen() to pass the fd as the client data instead of the file HANDLE (as in tif_unix.c). Then in all the other procedures, we convert the given fd back into a file HANDLE via _get_osfhandle() before proceeding as usual. This is efficient since it's just a table lookup (maintained by the CRT). This also allows us to remove all the AVOID_WIN32_FILEIO and USE_WIN32_FILEIO defines since they're no longer used.

The other thing about this change is that TIFFFdOpen() is now truly an "fd" open on Windows since it uses the same fd <-> HANDLE mapping as the CRT. This does break compatibility though, not only for TIFFFdOpen(), but also for TIFFFileno(), TIFFSetFileno(), and TIFFSetClientdata(). Previously, these functions expected a file HANDLE but now they're actually fd's. To get back the old functionality, external users will have to now call _get_osfhandle() and _open_osfhandle() first as appropriate. So this is the point where someone might say the patch is bad. On the plus side it makes everything match much closer to tif_unix.c. On the minus side, it breaks compatibility in a subtle way. Everything will still link but then crash if the libtiff windows user expected a HANDLE. The changes made to fax2ps.c and fax2tiff.c illustrate what I'm discussing here.

I've done code path testing of the patched tif_win32.c using a small RGBA image. For large (3 GB) image testing, I scaled the small image into a large image, verified it's tiffdump and tiffinfo output, scaled it back down to a small file, and then examined that the data is still correct. Unfortunately, I don't have a viewer capable of viewing 3 GB tiff files. :( I wasn't able to test the changes in fax2ps.c and fax2tiff.c but I think they should be ok.

-Edward

Index: nmake.opt

=================================================================== RCS file: /cvs/maptools/cvsroot/libtiff/nmake.opt,v retrieving revision 1.18

diff -p -u -r1.18 nmake.opt

--- nmake.opt   7 Jun 2006 16:33:45 -0000       1.18
+++ nmake.opt   13 Aug 2008 04:20:13 -0000

@@ -117,13 +117,6 @@ CHECK_JPEG_YCBCR_SUBSAMPLING = 1

 OPTFLAGS =     /Ox /MD /EHsc /W3 /D_CRT_SECURE_NO_DEPRECATE
 #OPTFLAGS =    /Zi 

-#

-# Uncomment following line to enable using Windows Common RunTime Library -# instead of Windows specific system calls. See notes on top of tif_unix.c

-# module for details.
-#
-USE_WIN_CRT_LIB = 1
-
 # Compiler specific options. You may probably want to adjust compilation
 # parameters in CFLAGS variable. Refer to your compiler documentation
 # for the option reference.
@@ -211,8 +204,3 @@ EXTRAFLAGS = -DDEFAULT_EXTRASAMPLE_AS_AL
 EXTRAFLAGS = -DCHECK_JPEG_YCBCR_SUBSAMPLING $(EXTRAFLAGS)
 !ENDIF

-!IFDEF USE_WIN_CRT_LIB
-EXTRAFLAGS = -DAVOID_WIN32_FILEIO $(EXTRAFLAGS)
-!ELSE
-EXTRAFLAGS = -DUSE_WIN32_FILEIO $(EXTRAFLAGS)
-!ENDIF
Index: libtiff/tif_win32.c

=================================================================== RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tif_win32.c,v retrieving revision 1.35

diff -p -u -r1.35 tif_win32.c
--- libtiff/tif_win32.c 17 Jun 2008 18:47:03 -0000 1.35
+++ libtiff/tif_win32.c 13 Aug 2008 04:20:14 -0000
@@ -31,6 +31,7 @@
 #include "tiffiop.h"

 #include <windows.h>
+#include <io.h>

 static tmsize_t
 _tiffReadProc(thandle_t fd, void* buf, tmsize_t size)

@@ -38,6 +39,7 @@ _tiffReadProc(thandle_t fd, void* buf, t
        /* tmsize_t is 64bit on 64bit systems, but the WinAPI ReadFile takes
         * 32bit sizes, so we loop through the data in suitable 32bit sized
         * chunks */

+       HANDLE hndl = (HANDLE)_get_osfhandle((int)fd);
        uint8* ma;
        uint64 mb;

        DWORD n;
@@ -51,7 +53,7 @@ _tiffReadProc(thandle_t fd, void* buf, t
                n=0x80000000UL;
                if ((uint64)n>mb)
                        n=(DWORD)mb;
- if (!ReadFile(fd,(LPVOID)ma,n,&o,NULL))
+ if (!ReadFile(hndl,(LPVOID)ma,n,&o,NULL))
                        return(0);
                ma+=o;
                mb-=o;

@@ -68,6 +70,7 @@ _tiffWriteProc(thandle_t fd, void* buf, 
        /* tmsize_t is 64bit on 64bit systems, but the WinAPI WriteFile takes
         * 32bit sizes, so we loop through the data in suitable 32bit sized
         * chunks */

+       HANDLE hndl = (HANDLE)_get_osfhandle((int)fd);
        uint8* ma;
        uint64 mb;

        DWORD n;
@@ -81,7 +84,7 @@ _tiffWriteProc(thandle_t fd, void* buf,
                n=0x80000000UL;
                if ((uint64)n>mb)
                        n=(DWORD)mb;
- if (!WriteFile(fd,(LPVOID)ma,n,&o,NULL))
+ if (!WriteFile(hndl,(LPVOID)ma,n,&o,NULL))
                        return(0);
                ma+=o;
                mb-=o;
@@ -95,6 +98,7 @@ _tiffWriteProc(thandle_t fd, void* buf,
 static uint64
 _tiffSeekProc(thandle_t fd, uint64 off, int whence)
 {
+ HANDLE hndl = (HANDLE)_get_osfhandle((int)fd);
        LARGE_INTEGER offli;
        DWORD dwMoveMethod;
        offli.QuadPart = off;
@@ -113,7 +117,7 @@ _tiffSeekProc(thandle_t fd, uint64 off,
                        dwMoveMethod = FILE_BEGIN;
                        break;
        }
- offli.LowPart=SetFilePointer(fd,offli.LowPart,&offli.HighPart,dwMoveMethod);
+ offli.LowPart=SetFilePointer(hndl,offli.LowPart,&offli.HighPart,dwMoveMethod);
        if ((offli.LowPart==INVALID_SET_FILE_POINTER)&&(GetLastError()!=NO_ERROR))
                offli.QuadPart=0;
        return(offli.QuadPart);
@@ -122,14 +126,16 @@ _tiffSeekProc(thandle_t fd, uint64 off,
 static int
 _tiffCloseProc(thandle_t fd)
 {

-       return (CloseHandle(fd) ? 0 : -1);
+       // _close() will also close the underlying HANDLE
+       return (_close((int)fd) ? 0 : -1);

 }

 static uint64
 _tiffSizeProc(thandle_t fd)
 {

+       HANDLE hndl = (HANDLE)_get_osfhandle((int)fd);
        ULARGE_INTEGER m;
-       m.LowPart=GetFileSize(fd,&m.HighPart);
+       m.LowPart=GetFileSize(hndl,&m.HighPart);

        return(m.QuadPart);
 }

@@ -156,6 +162,7 @@ _tiffDummyMapProc(thandle_t fd, void** p
 static int
 _tiffMapProc(thandle_t fd, void** pbase, toff_t* psize)
 {
+ HANDLE hndl = (HANDLE)_get_osfhandle((int)fd);
        uint64 size;
        tmsize_t sizem;
        HANDLE hMapFile;
@@ -167,7 +174,7 @@ _tiffMapProc(thandle_t fd, void** pbase,

        // By passing in 0 for the maximum file size, it specifies that we
        // create a file mapping object for the full file size.

-       hMapFile = CreateFileMapping(fd, NULL, PAGE_READONLY, 0, 0, NULL);
+       hMapFile = CreateFileMapping(hndl, NULL, PAGE_READONLY, 0, 0, NULL);

        if (hMapFile == NULL)
                return (0);
        *pbase = MapViewOfFile(hMapFile, FILE_MAP_READ, 0, 0, 0);
@@ -233,7 +240,8 @@ TIFF*
 TIFFOpen(const char* name, const char* mode)
 {
        static const char module[] = "TIFFOpen";
- thandle_t fd;
+ HANDLE hndl;
+ int fd;
        int m;
        DWORD dwMode;
        TIFF* tif;
@@ -249,19 +257,25 @@ TIFFOpen(const char* name, const char* m
                default: return ((TIFF*)0);
        }

-       fd = (thandle_t)CreateFileA(name,
+       hndl = CreateFileA(name,

                (m == O_RDONLY)?GENERIC_READ:(GENERIC_READ | GENERIC_WRITE),
                FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, dwMode,
                (m == O_RDONLY)?FILE_ATTRIBUTE_READONLY:FILE_ATTRIBUTE_NORMAL,
                NULL);
- if (fd == INVALID_HANDLE_VALUE) {
+ if (hndl == INVALID_HANDLE_VALUE) {
                TIFFErrorExt(0, module, "%s: Cannot open", name);
                return ((TIFF *)0);
        }

-       tif = TIFFFdOpen((int)fd, name, mode);
+       fd = _open_osfhandle((intptr_t)hndl, (m | _O_BINARY));
+       if (fd < 0) {
+               TIFFErrorExt(0, module, "%s: Cannot create fd", name);
+               return ((TIFF *)0);
+       }
+
+       tif = TIFFFdOpen(fd, name, mode);
        if(!tif)
-               CloseHandle(fd);
+               _close(fd);     // this will close the underlying hndl as well

        return tif;
 }

@@ -272,7 +286,8 @@ TIFF*
 TIFFOpenW(const wchar_t* name, const char* mode)
 {
        static const char module[] = "TIFFOpenW";
- thandle_t fd;
+ HANDLE hndl;
+ int fd;
        int m;
        DWORD dwMode;
        int mbsize;
@@ -290,12 +305,12 @@ TIFFOpenW(const wchar_t* name, const cha
                default: return ((TIFF*)0);
        }

-       fd = (thandle_t)CreateFileW(name,
+       hndl = CreateFileW(name,

                (m == O_RDONLY)?GENERIC_READ:(GENERIC_READ|GENERIC_WRITE),
                FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, dwMode,
                (m == O_RDONLY)?FILE_ATTRIBUTE_READONLY:FILE_ATTRIBUTE_NORMAL,
                NULL);
- if (fd == INVALID_HANDLE_VALUE) {
+ if (hndl == INVALID_HANDLE_VALUE) {
                TIFFErrorExt(0, module, "%S: Cannot open", name);
                return ((TIFF *)0);
        }
@@ -314,10 +329,15 @@ TIFFOpenW(const wchar_t* name, const cha
                                    NULL, NULL);
        }

+       fd = _open_osfhandle((intptr_t)hndl, (m | _O_BINARY));
+       if (fd < 0) {
+               TIFFErrorExt(0, module, "%s: Cannot create fd", name);
+               return ((TIFF *)0);
+       }
+
+       tif = TIFFFdOpen(fd, (mbname != NULL) ? mbname : "<unknown>", mode);
        if(!tif)
-               CloseHandle(fd);
+               _close(fd);     // this will close the underlying hndl as well

        _TIFFfree(mbname);

Index: libtiff/tiffio.h

=================================================================== RCS file: /cvs/maptools/cvsroot/libtiff/libtiff/tiffio.h,v retrieving revision 1.81

diff -p -u -r1.81 tiffio.h

--- libtiff/tiffio.h    10 Apr 2008 11:08:48 -0000      1.81
+++ libtiff/tiffio.h    13 Aug 2008 04:20:14 -0000

@@ -79,30 +79,7 @@ typedef uint64 toff_t;          /* file 

 #define __WIN32__
 #endif

-/*

- */
-

-#if defined(_WINDOWS) || defined(__WIN32__) || defined(_Windows) -# if !defined(__CYGWIN) && !defined(AVOID_WIN32_FILEIO) && !defined(USE_WIN32_FILEIO)

-# define AVOID_WIN32_FILEIO
-# endif
-#endif
-
-#if defined(USE_WIN32_FILEIO)
-# define VC_EXTRALEAN
-# include <windows.h>
-# ifdef __WIN32__
-DECLARE_HANDLE(thandle_t); /* Win32 file handle */
-# else
-typedef HFILE thandle_t; /* client data handle */
-# endif /* __WIN32__ */
-#else
 typedef void* thandle_t; /* client data handle */
-#endif /* USE_WIN32_FILEIO */

 /*
  * Flags to pass to TIFFPrintDirectory to control
Index: tools/fax2ps.c

===================================================================
RCS file: /cvs/maptools/cvsroot/libtiff/tools/fax2ps.c,v
retrieving revision 1.22
diff -p -u -r1.22 fax2ps.c
--- tools/fax2ps.c      20 Apr 2006 12:36:23 -0000      1.22
+++ tools/fax2ps.c      13 Aug 2008 04:20:14 -0000
@@ -390,11 +390,7 @@ main(int argc, char** argv)
        while ((n = read(fileno(stdin), buf, sizeof (buf))) > 0)
            write(fileno(fd), buf, n);
        lseek(fileno(fd), 0, SEEK_SET);
-#if defined(_WIN32) && defined(USE_WIN32_FILEIO)
-       tif = TIFFFdOpen(_get_osfhandle(fileno(fd)), "temp", "r");
-#else
        tif = TIFFFdOpen(fileno(fd), "temp", "r");
-#endif
        if (tif) {
            fax2ps(tif, npages, pages, "<stdin>");
            TIFFClose(tif);
Index: tools/fax2tiff.c
===================================================================

RCS file: /cvs/maptools/cvsroot/libtiff/tools/fax2tiff.c,v retrieving revision 1.19

diff -p -u -r1.19 fax2tiff.c

--- tools/fax2tiff.c    20 Apr 2006 12:36:23 -0000      1.19
+++ tools/fax2tiff.c    13 Aug 2008 04:20:14 -0000

@@ -260,11 +260,7 @@ main(int argc, char* argv[])
                            "%s: %s: Can not open\n", argv[0], argv[optind]);
                        continue;
                }
-#if defined(_WIN32) && defined(USE_WIN32_FILEIO)
- TIFFSetClientdata(faxTIFF, (thandle_t)_get_osfhandle(fileno(in)));
-#else
                 TIFFSetClientdata(faxTIFF, (thandle_t)fileno(in));
-#endif
                TIFFSetFileName(faxTIFF, (const char*)argv[optind]);
                TIFFSetField(out, TIFFTAG_IMAGEWIDTH, xsize);
                TIFFSetField(out, TIFFTAG_BITSPERSAMPLE, 1);