From a923e21631a29dc8b8781d7d02b5003d0df64ca3 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 12 Apr 2023 09:09:38 +0900 Subject: [PATCH] Fix detection of unseekable files for fseek() and ftello() with MSVC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calling fseek() or ftello() on a handle to a non-seeking device such as a pipe or a communications device is not supported. Unfortunately, MSVC's flavor of these routines, _fseeki64() and _ftelli64(), do not return an error when given a pipe as handle. Some of the logic of pg_dump and restore relies on these routines to check if a handle is seekable, causing failures when passing the contents of pg_dump to pg_restore through a pipe, for example. This commit introduces wrappers for fseeko() and ftello() on MSVC so as any callers are able to properly detect the cases of non-seekable handles. This relies mainly on GetFileType(), sharing a bit of code with the MSVC port for fstat(). The code in charge of getting a file type is refactored into a new file called win32common.c, shared by win32stat.c and the new win32fseek.c. It includes the MSVC ports for fseeko() and ftello(). Like 765f5df, this is backpatched down to 14, where the fstat() implementation for MSVC is able to understand about files larger than 4GB in size. Using a TAP test for that is proving to be tricky as IPC::Run handles the pipes by itself, still I have been able to check the fix manually. Reported-by: Daniel Watzinger Author: Juan José Santamaría Flecha, Michael Paquier Discussion: https://postgr.es/m/CAC+AXB26a4EmxM2suXxPpJaGrqAdxracd7hskLg-zxtPB50h7A@mail.gmail.com Backpatch-through: 14 --- configure | 6 +++ configure.ac | 1 + src/include/port/win32_port.h | 12 ++++-- src/port/meson.build | 2 + src/port/win32common.c | 68 +++++++++++++++++++++++++++++++ src/port/win32fseek.c | 75 +++++++++++++++++++++++++++++++++++ src/port/win32stat.c | 22 ++-------- src/tools/msvc/Mkvcbuild.pm | 2 + 8 files changed, 166 insertions(+), 22 deletions(-) create mode 100644 src/port/win32common.c create mode 100644 src/port/win32fseek.c diff --git a/configure b/configure index 905be9568b..dbea7eaf5f 100755 --- a/configure +++ b/configure @@ -16492,6 +16492,12 @@ esac ;; esac + case " $LIBOBJS " in + *" win32common.$ac_objext "* ) ;; + *) LIBOBJS="$LIBOBJS win32common.$ac_objext" + ;; +esac + case " $LIBOBJS " in *" win32dlopen.$ac_objext "* ) ;; *) LIBOBJS="$LIBOBJS win32dlopen.$ac_objext" diff --git a/configure.ac b/configure.ac index 8095dfcf1d..dda34304db 100644 --- a/configure.ac +++ b/configure.ac @@ -1894,6 +1894,7 @@ if test "$PORTNAME" = "win32"; then AC_LIBOBJ(kill) AC_LIBOBJ(open) AC_LIBOBJ(system) + AC_LIBOBJ(win32common) AC_LIBOBJ(win32dlopen) AC_LIBOBJ(win32env) AC_LIBOBJ(win32error) diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h index 5116c2fc06..58965e0dfd 100644 --- a/src/include/port/win32_port.h +++ b/src/include/port/win32_port.h @@ -204,15 +204,21 @@ struct itimerval int setitimer(int which, const struct itimerval *value, struct itimerval *ovalue); +/* Convenience wrapper for GetFileType() */ +extern DWORD pgwin32_get_file_type(HANDLE hFile); + /* * WIN32 does not provide 64-bit off_t, but does provide the functions operating - * with 64-bit offsets. + * with 64-bit offsets. Also, fseek() might not give an error for unseekable + * streams, so harden that function with our version. */ #define pgoff_t __int64 #ifdef _MSC_VER -#define fseeko(stream, offset, origin) _fseeki64(stream, offset, origin) -#define ftello(stream) _ftelli64(stream) +extern int _pgfseeko64(FILE *stream, pgoff_t offset, int origin); +extern pgoff_t _pgftello64(FILE *stream); +#define fseeko(stream, offset, origin) _pgfseeko64(stream, offset, origin) +#define ftello(stream) _pgftello64(stream) #else #ifndef fseeko #define fseeko(stream, offset, origin) fseeko64(stream, offset, origin) diff --git a/src/port/meson.build b/src/port/meson.build index b174b25021..24416b9bfc 100644 --- a/src/port/meson.build +++ b/src/port/meson.build @@ -29,10 +29,12 @@ if host_system == 'windows' 'kill.c', 'open.c', 'system.c', + 'win32common.c', 'win32dlopen.c', 'win32env.c', 'win32error.c', 'win32fdatasync.c', + 'win32fseek.c', 'win32getrusage.c', 'win32link.c', 'win32ntdll.c', diff --git a/src/port/win32common.c b/src/port/win32common.c new file mode 100644 index 0000000000..2fd78f7f93 --- /dev/null +++ b/src/port/win32common.c @@ -0,0 +1,68 @@ +/*------------------------------------------------------------------------- + * + * win32common.c + * Common routines shared among the win32*.c ports. + * + * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * + * IDENTIFICATION + * src/port/win32common.c + * + *------------------------------------------------------------------------- + */ + +#ifdef FRONTEND +#include "postgres_fe.h" +#else +#include "postgres.h" +#endif + +#ifdef WIN32 + +/* + * pgwin32_get_file_type + * + * Convenience wrapper for GetFileType() with specific error handling for all the + * port implementations. Returns the file type associated with a HANDLE. + * + * On error, sets errno with FILE_TYPE_UNKNOWN as file type. + */ +DWORD +pgwin32_get_file_type(HANDLE hFile) +{ + DWORD fileType = FILE_TYPE_UNKNOWN; + DWORD lastError; + + errno = 0; + + /* + * When stdin, stdout, and stderr aren't associated with a stream the + * special value -2 is returned: + * https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle + */ + if (hFile == INVALID_HANDLE_VALUE || hFile == (HANDLE) -2) + { + errno = EINVAL; + return FILE_TYPE_UNKNOWN; + } + + fileType = GetFileType(hFile); + lastError = GetLastError(); + + /* + * Invoke GetLastError in order to distinguish between a "valid" return of + * FILE_TYPE_UNKNOWN and its return due to a calling error. In case of + * success, GetLastError() returns NO_ERROR. + */ + if (fileType == FILE_TYPE_UNKNOWN && lastError != NO_ERROR) + { + _dosmaperr(lastError); + return FILE_TYPE_UNKNOWN; + } + + return fileType; +} + +#endif /* WIN32 */ diff --git a/src/port/win32fseek.c b/src/port/win32fseek.c new file mode 100644 index 0000000000..985313c825 --- /dev/null +++ b/src/port/win32fseek.c @@ -0,0 +1,75 @@ +/*------------------------------------------------------------------------- + * + * win32fseek.c + * Replacements for fseeko() and ftello(). + * + * Portions Copyright (c) 1996-2023, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/port/win32fseek.c + * + *------------------------------------------------------------------------- + */ + +#ifdef FRONTEND +#include "postgres_fe.h" +#else +#include "postgres.h" +#endif + +#if defined(WIN32) && defined(_MSC_VER) + +/* + * _pgfseeko64 + * + * Calling fseek() on a handle to a non-seeking device such as a pipe or + * a communications device is not supported, and fseek() may not return + * an error. This wrapper relies on the file type to check which cases + * are supported. + */ +int +_pgfseeko64(FILE *stream, pgoff_t offset, int origin) +{ + DWORD fileType; + HANDLE hFile = (HANDLE) _get_osfhandle(_fileno(stream)); + + fileType = pgwin32_get_file_type(hFile); + if (errno != 0) + return -1; + + if (fileType == FILE_TYPE_DISK) + return _fseeki64(stream, offset, origin); + else if (fileType == FILE_TYPE_CHAR || fileType == FILE_TYPE_PIPE) + errno = ESPIPE; + else + errno = EINVAL; + + return -1; +} + +/* + * _pgftello64 + * + * Same as _pgfseeko64(). + */ +pgoff_t +_pgftello64(FILE *stream) +{ + DWORD fileType; + HANDLE hFile = (HANDLE) _get_osfhandle(_fileno(stream)); + + fileType = pgwin32_get_file_type(hFile); + if (errno != 0) + return -1; + + if (fileType == FILE_TYPE_DISK) + return _ftelli64(stream); + else if (fileType == FILE_TYPE_CHAR || fileType == FILE_TYPE_PIPE) + errno = ESPIPE; + else + errno = EINVAL; + + return -1; +} + +#endif /* defined(WIN32) && defined(_MSC_VER) */ diff --git a/src/port/win32stat.c b/src/port/win32stat.c index b79da738b2..aa3a0c174e 100644 --- a/src/port/win32stat.c +++ b/src/port/win32stat.c @@ -258,33 +258,17 @@ _pgfstat64(int fileno, struct stat *buf) { HANDLE hFile = (HANDLE) _get_osfhandle(fileno); DWORD fileType = FILE_TYPE_UNKNOWN; - DWORD lastError; unsigned short st_mode; - /* - * When stdin, stdout, and stderr aren't associated with a stream the - * special value -2 is returned: - * https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/get-osfhandle - */ - if (hFile == INVALID_HANDLE_VALUE || hFile == (HANDLE) -2 || buf == NULL) + if (buf == NULL) { errno = EINVAL; return -1; } - fileType = GetFileType(hFile); - lastError = GetLastError(); - - /* - * Invoke GetLastError in order to distinguish between a "valid" return of - * FILE_TYPE_UNKNOWN and its return due to a calling error. In case of - * success, GetLastError returns NO_ERROR. - */ - if (fileType == FILE_TYPE_UNKNOWN && lastError != NO_ERROR) - { - _dosmaperr(lastError); + fileType = pgwin32_get_file_type(hFile); + if (errno != 0) return -1; - } switch (fileType) { diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index 3d1e0041b2..958206f315 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -108,9 +108,11 @@ sub mkvcbuild pg_strong_random.c pgcheckdir.c pgmkdirp.c pgsleep.c pgstrcasecmp.c pqsignal.c mkdtemp.c qsort.c qsort_arg.c bsearch_arg.c quotes.c system.c strerror.c tar.c + win32common.c win32dlopen.c win32env.c win32error.c win32fdatasync.c + win32fseek.c win32getrusage.c win32gettimeofday.c win32link.c