Re: BUG #13774: upgrade from 9.1 to 9.4 'succeeds' without enough disk space

On Tue, Nov 17, 2015 at 11:43:34PM -0500, Noah Misch wrote:
> On Fri, Nov 13, 2015 at 08:24:02PM -0500, Bruce Momjian wrote:
> > Thank you for the report. This is embarrassing, but the code was
> > testing for the wrong return value on Windows. We used a macro to
> > define the same symbol on Windows and Unix (pg_copy_file)
>
> > *** a/src/bin/pg_upgrade/file.c
> > --- b/src/bin/pg_upgrade/file.c
> > *************** copyAndUpdateFile(pageCnvCtx *pageConver
> > *** 34,40 ****
> > {
> > if (pageConverter == NULL)
> > {
> > ! if (pg_copy_file(src, dst, force) == -1)
> > return getErrorText(errno);
> > else
> > return NULL;
> > --- 34,44 ----
> > {
> > if (pageConverter == NULL)
> > {
> > ! #ifndef WIN32
> > ! if (copy_file(src, dst, force) == -1)
> > ! #else
> > ! if (CopyFile(src, dst, force) == 0)
> > ! #endif
> > return getErrorText(errno);
>
> Thanks. This passage of code has at least two other problems on Windows. The
> third CopyFile() argument means the opposite of the third copy_file()
> argument. Next, getErrorText() is wrong on Windows:
>
> const char *
> getErrorText(int errNum)
> {
> #ifdef WIN32
> _dosmaperr(GetLastError());
> #endif
> return pg_strdup(strerror(errNum));
> }
>
> Calling _dosmaperr() to set errno is futile, because we nonetheless proceed to
> use the passed-in errNum.

Very good points. The attached patch fixes the copy parameter and
addresses the errno issue by just using errno direcdtly after the
_dosmaperr call.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription +