From 90ca64668d530318544c6ddb28fb4237b2705672 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Sun, 4 Nov 2018 19:51:36 +0000 Subject: [PATCH 1/3] Fallback to standard temp file if O_TMPFILE fails #1151 --- libvips/iofuncs/vips.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 7cabaff0..2c59f807 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -188,6 +188,7 @@ vips__open_image_write( const char *filename, gboolean temp ) int fd; flags = MODE_WRITE; + fd = -1; #ifdef O_TMPFILE /* Linux-only extension creates an unlinked file. CREAT and TRUNC must @@ -196,18 +197,11 @@ vips__open_image_write( const char *filename, gboolean temp ) if( temp ) { char *dirname; - flags |= O_TMPFILE; - flags &= ~O_CREAT; - flags &= ~O_TRUNC; - dirname = g_path_get_dirname( filename ); - fd = vips_tracked_open( dirname, flags, 0666 ); + fd = vips_tracked_open( dirname, O_TMPFILE | O_RDWR , 0666 ); g_free( dirname ); } - else - fd = vips_tracked_open( filename, flags, 0666 ); -#else /*!O_TMPFILE*/ - fd = vips_tracked_open( filename, flags, 0666 ); +#endif /*O_TMPFILE*/ #ifdef _O_TEMPORARY /* On Windows, setting _O_TEMPORARY gets the file automatically @@ -217,7 +211,9 @@ vips__open_image_write( const char *filename, gboolean temp ) if( temp ) flags |= _O_TEMPORARY; #endif /*_O_TEMPORARY*/ -#endif /*O_TMPFILE*/ + + if( fd < 0 ) + fd = vips_tracked_open( filename, flags, 0666 ); if( fd < 0 ) { vips_error_system( errno, "VipsImage", From 89c6d290799428717256d663d9e5fc25eb192a9e Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 5 Nov 2018 10:38:15 +0000 Subject: [PATCH 2/3] add comments re. temp file open bug --- libvips/iofuncs/image.c | 8 ++++---- libvips/iofuncs/vips.c | 13 ++++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/libvips/iofuncs/image.c b/libvips/iofuncs/image.c index e4489778..99893a11 100644 --- a/libvips/iofuncs/image.c +++ b/libvips/iofuncs/image.c @@ -3077,13 +3077,13 @@ vips_image_rewind_output( VipsImage *image ) /* Now we've finished writing and reopened as read, we can * delete-on-close. * - * On *nix-like systems, this will unlink the file - * from the filesystem and when we exit, for whatever reason, the file + * On *nix-like systems, this will unlink the file from the + * filesystem and when we exit, for whatever reason, the file * we be reclaimed. * * On Windows this will fail because the file is open and you can't - * delete open files. However, on Windows we set O_TEMP, so the file - * will be deleted when the fd is finally closed. + * delete open files. However, on Windows we set _O_TEMPORARY, so the + * file will be deleted when the fd is finally closed. */ vips_image_delete( image ); diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 2c59f807..e7d71378 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -137,7 +137,6 @@ * * We use O_RDWR not O_WRONLY since after writing we may want to rewind the * image and read from it. - * */ #define MODE_WRITE BINARYIZE (O_RDWR | O_CREAT | O_TRUNC) @@ -187,12 +186,15 @@ vips__open_image_write( const char *filename, gboolean temp ) int flags; int fd; - flags = MODE_WRITE; fd = -1; #ifdef O_TMPFILE /* Linux-only extension creates an unlinked file. CREAT and TRUNC must * be clear. The filename arg to open() must name a directory. + * + * This can fail since not all filesystems support it. In this case, + * we open as a regular file and rely on the delete-on-close + * mechanism, see vips_image_delete(). */ if( temp ) { char *dirname; @@ -203,10 +205,11 @@ vips__open_image_write( const char *filename, gboolean temp ) } #endif /*O_TMPFILE*/ + flags = MODE_WRITE; + #ifdef _O_TEMPORARY - /* On Windows, setting _O_TEMPORARY gets the file automatically - * deleted on process exit, even if the processes crashes. See - * vips_image_rewind() for what we do to help on *nix. + /* On Windows, setting _O_TEMPORARY will delete the file automatically + * on process exit, even if the processes crashes. */ if( temp ) flags |= _O_TEMPORARY; From 0bea76d3645f27b9cb3c5067d51e752d62812de5 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 5 Nov 2018 10:40:51 +0000 Subject: [PATCH 3/3] fix temp file open was not auto-deleteing on windows, needs a fallback on linux see https://github.com/libvips/libvips/pull/1155 --- ChangeLog | 1 + libvips/iofuncs/vips.c | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index a7b86a57..bedc0fa8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ 23/9/18 started 8.7.1 - update function list in docs [janko-m] - test for g_str_to_ascii() [jcupitt] +- fix temp file open on Windows and fallback on linux [lovell] 23/12/17 started 8.7.0 - add magicksave, save image with libMagick [dlemstra] diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 7cabaff0..c224bdba 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -187,37 +187,37 @@ vips__open_image_write( const char *filename, gboolean temp ) int flags; int fd; - flags = MODE_WRITE; + fd = -1; #ifdef O_TMPFILE /* Linux-only extension creates an unlinked file. CREAT and TRUNC must * be clear. The filename arg to open() must name a directory. + * + * This can fail since not all filesystems support it. In this case, + * we open as a regular file and rely on the delete-on-close + * mechanism, see vips_image_delete(). */ if( temp ) { char *dirname; - flags |= O_TMPFILE; - flags &= ~O_CREAT; - flags &= ~O_TRUNC; - dirname = g_path_get_dirname( filename ); - fd = vips_tracked_open( dirname, flags, 0666 ); + fd = vips_tracked_open( dirname, O_TMPFILE | O_RDWR , 0666 ); g_free( dirname ); } - else - fd = vips_tracked_open( filename, flags, 0666 ); -#else /*!O_TMPFILE*/ - fd = vips_tracked_open( filename, flags, 0666 ); +#endif /*O_TMPFILE*/ + + flags = MODE_WRITE; #ifdef _O_TEMPORARY - /* On Windows, setting _O_TEMPORARY gets the file automatically - * deleted on process exit, even if the processes crashes. See - * vips_image_rewind() for what we do to help on *nix. + /* On Windows, setting _O_TEMPORARY will delete the file automatically + * on process exit, even if the processes crashes. */ if( temp ) flags |= _O_TEMPORARY; #endif /*_O_TEMPORARY*/ -#endif /*O_TMPFILE*/ + + if( fd < 0 ) + fd = vips_tracked_open( filename, flags, 0666 ); if( fd < 0 ) { vips_error_system( errno, "VipsImage",