pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedpg_dump -Ft failed on Windows XP
22+ messages / 7 participants
[nested] [flat]
* pg_dump -Ft failed on Windows XP
@ 2006-04-20 04:06 Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
0 siblings, 0 replies; 22+ messages in thread
From: Yoshiyuki Asaba @ 2006-04-20 04:06 UTC (permalink / raw)
To: pgsql-hackers
Hi,
I got the following message when I ran pg_dump with -Ft option on
Windows XP.
> pg_dump -V
pg_dump (PostgreSQL) 8.1.2
> pg_dump -Ft test > C:\backup\xxx.out
pg_dump: [tar archiver] could not generate temporary file name: Permission denied
pg_dump calls tmpfile() in pg_backup_tar.c:tarOpen(). Win32's
tmpfile() creates the file into root folder. But non-administrator
users can't create files into root folder. So, I think it fails that
non-administrator users run pg_dump with -Ft option.
Regards,
--
Yoshiyuki Asaba
y-asaba@sraoss.co.jp
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 08:47 Magnus Hagander <mha@sollentuna.net>
0 siblings, 2 replies; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 08:47 UTC (permalink / raw)
To: Yoshiyuki Asaba <y-asaba@sraoss.co.jp>; pgsql-hackers
> I got the following message when I ran pg_dump with -Ft
> option on Windows XP.
>
> > pg_dump -V
> pg_dump (PostgreSQL) 8.1.2
>
> > pg_dump -Ft test > C:\backup\xxx.out
> pg_dump: [tar archiver] could not generate temporary file
> name: Permission denied
>
>
> pg_dump calls tmpfile() in pg_backup_tar.c:tarOpen(). Win32's
> tmpfile() creates the file into root folder. But
> non-administrator users can't create files into root folder.
> So, I think it fails that non-administrator users run pg_dump
> with -Ft option.
Indeed, that's definitly a bug. Quick patch attached. It does appear to
work, but there may be a better way?
//Magnus
Attachments:
[application/octet-stream] pg_dump_tempfile.patch (1.8K, ../../6BCB9D8A16AC4241919521715F4D8BCEA352CE@algol.sollentuna.se/2-pg_dump_tempfile.patch)
download | inline diff:
Index: bin/pg_dump/pg_backup_tar.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.50
diff -c -r1.50 pg_backup_tar.c
*** bin/pg_dump/pg_backup_tar.c 12 Feb 2006 06:11:50 -0000 1.50
--- bin/pg_dump/pg_backup_tar.c 20 Apr 2006 08:45:03 -0000
***************
*** 68,73 ****
--- 68,74 ----
FILE *tarFH;
FILE *tmpFH;
char *targetFile;
+ char *tempFile;
char mode;
off_t pos;
off_t fileLen;
***************
*** 360,370 ****
}
else
{
tm = calloc(1, sizeof(TAR_MEMBER));
! tm->tmpFH = tmpfile();
! if (tm->tmpFH == NULL)
die_horribly(AH, modulename, "could not generate temporary file name: %s\n", strerror(errno));
#ifdef HAVE_LIBZ
--- 361,378 ----
}
else
{
+ char *tmpname;
tm = calloc(1, sizeof(TAR_MEMBER));
! tmpname = tempnam(NULL, NULL);
! if (tmpname != NULL)
! {
! tm->tempFile = strdup(tmpname);
! if (tm->tempFile != NULL)
! tm->tmpFH = fopen(tm->tempFile , "w+b");
! }
! if (tmpname == NULL || tm->tempFile == NULL || tm->tmpFH == NULL)
die_horribly(AH, modulename, "could not generate temporary file name: %s\n", strerror(errno));
#ifdef HAVE_LIBZ
***************
*** 1040,1047 ****
len += res;
}
! if (fclose(tmp) != 0) /* This *should* delete it... */
die_horribly(AH, modulename, "could not close tar member: %s\n", strerror(errno));
if (len != th->fileLen)
{
--- 1048,1060 ----
len += res;
}
! if (fclose(tmp) != 0)
die_horribly(AH, modulename, "could not close tar member: %s\n", strerror(errno));
+ if (th->tempFile != NULL)
+ {
+ unlink(th->tempFile);
+ free(th->tempFile);
+ }
if (len != th->fileLen)
{
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 10:45 Peter Eisentraut <peter_e@gmx.net>
parent: Magnus Hagander <mha@sollentuna.net>
1 sibling, 0 replies; 22+ messages in thread
From: Peter Eisentraut @ 2006-04-20 10:45 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Magnus Hagander <mha@sollentuna.net>; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
Am Donnerstag, 20. April 2006 10:47 schrieb Magnus Hagander:
> Indeed, that's definitly a bug. Quick patch attached. It does appear to
> work, but there may be a better way?
This patch introduces a security hole because an attacker could create, say, a
suitable symlink between the time the name is generated and the file is
opened.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 11:03 Magnus Hagander <mha@sollentuna.net>
0 siblings, 1 reply; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 11:03 UTC (permalink / raw)
To: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers; +Cc: Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> > Indeed, that's definitly a bug. Quick patch attached. It
> does appear
> > to work, but there may be a better way?
>
> This patch introduces a security hole because an attacker
> could create, say, a suitable symlink between the time the
> name is generated and the file is opened.
Good point. I guess what I need to do is use open() specifying O_CREATE,
and then fdopen() that file.
Question: Is the use of O_TEMPORARY to open() portable? (my win32 docs
say it will make the file automatically deleted when the last descriptor
is closed, which I didn't know before. That would make the patch much
simpler, but might require #ifdefs?)
//Magnus
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 11:19 Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 11:19 UTC (permalink / raw)
To: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers; +Cc: Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> > > Indeed, that's definitly a bug. Quick patch attached. It
> > does appear
> > > to work, but there may be a better way?
> >
> > This patch introduces a security hole because an attacker could
> > create, say, a suitable symlink between the time the name
> is generated
> > and the file is opened.
>
> Good point. I guess what I need to do is use open()
> specifying O_CREATE, and then fdopen() that file.
>
> Question: Is the use of O_TEMPORARY to open() portable? (my
> win32 docs say it will make the file automatically deleted
> when the last descriptor is closed, which I didn't know
> before. That would make the patch much simpler, but might
> require #ifdefs?)
Actually, since I'm running out the door, here is a new attempt that
changes behaviour only on win32. And that also appears to work, but may
be wrong ;-)
//Magnus
Attachments:
[application/octet-stream] pg_dump_tempfile.patch (1.2K, ../../6BCB9D8A16AC4241919521715F4D8BCEA352D1@algol.sollentuna.se/2-pg_dump_tempfile.patch)
download | inline diff:
Index: bin/pg_dump/pg_backup_tar.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.50
diff -c -r1.50 pg_backup_tar.c
*** bin/pg_dump/pg_backup_tar.c 12 Feb 2006 06:11:50 -0000 1.50
--- bin/pg_dump/pg_backup_tar.c 20 Apr 2006 11:17:42 -0000
***************
*** 362,368 ****
--- 362,388 ----
{
tm = calloc(1, sizeof(TAR_MEMBER));
+ #ifndef WIN32
tm->tmpFH = tmpfile();
+ #else
+ /* on win32, tmpfile() generates a filename in the root directory, which requires
+ * administrative permissions to write to. */
+ while (1)
+ {
+ char *tmpname;
+ int fd;
+
+ tmpname = tempnam(NULL, NULL);
+ if (tmpname == NULL)
+ break;
+ fd = _open(tmpname, _O_RDWR | _O_CREAT | _O_EXCL | _O_TEMPORARY, 0);
+ if (fd == -1 && errno == EEXIST)
+ continue; /* Try again with a new name if file exists */
+ if (fd != -1)
+ tm->tmpFH = fdopen(fd, "w+b");
+ break;
+ }
+ #endif
if (tm->tmpFH == NULL)
die_horribly(AH, modulename, "could not generate temporary file name: %s\n", strerror(errno));
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 11:19 Peter Eisentraut <peter_e@gmx.net>
parent: Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 22+ messages in thread
From: Peter Eisentraut @ 2006-04-20 11:19 UTC (permalink / raw)
To: Magnus Hagander <mha@sollentuna.net>; +Cc: pgsql-hackers@postgresql.org, "Yoshiyuki Asaba" <y-asaba@sraoss.co.jp>
Am Donnerstag, 20. April 2006 13:03 schrieb Magnus Hagander:
> Question: Is the use of O_TEMPORARY to open() portable? (my win32 docs
> say it will make the file automatically deleted when the last descriptor
> is closed, which I didn't know before. That would make the patch much
> simpler, but might require #ifdefs?)
I think it would be more elegant if you wrote a replacement implementation of
tmpfile() for pgport and did not change pg_dump at all. And/or write a bug
to Microsoft about a buggy C library. :)
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 11:21 Magnus Hagander <mha@sollentuna.net>
0 siblings, 1 reply; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 11:21 UTC (permalink / raw)
To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> > Question: Is the use of O_TEMPORARY to open() portable? (my
> win32 docs
> > say it will make the file automatically deleted when the last
> > descriptor is closed, which I didn't know before. That
> would make the
> > patch much simpler, but might require #ifdefs?)
>
> I think it would be more elegant if you wrote a replacement
> implementation of
> tmpfile() for pgport and did not change pg_dump at all.
> And/or write a bug to Microsoft about a buggy C library. :)
It's not buggy. It's well documented behaviour,and per my linux manpage
for the file it's also OK per spec:
The standard does not specify the directory that tmpfile()
will use. Glibc will try the path prefix P_tmpdir defined
in <stdio.h>, and if that fails the directory /tmp.
It's certainly *stupid*, but not buggy ;-)
//Magnus
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 11:53 Peter Eisentraut <peter_e@gmx.net>
parent: Magnus Hagander <mha@sollentuna.net>
0 siblings, 1 reply; 22+ messages in thread
From: Peter Eisentraut @ 2006-04-20 11:53 UTC (permalink / raw)
To: Magnus Hagander <mha@sollentuna.net>; +Cc: pgsql-hackers@postgresql.org, "Yoshiyuki Asaba" <y-asaba@sraoss.co.jp>
Am Donnerstag, 20. April 2006 13:21 schrieb Magnus Hagander:
> It's not buggy. It's well documented behaviour,and per my linux manpage
> for the file it's also OK per spec:
>
> The standard does not specify the directory that tmpfile()
> will use. Glibc will try the path prefix P_tmpdir defined
> in <stdio.h>, and if that fails the directory /tmp.
The spec says
The tmpfile() function shall create a temporary file and open a
corresponding stream. The file shall be automatically deleted when all
references to the file are closed. The file is opened as in fopen() for
update (w+).
If the implementation is such that it tries to create the file in a directory
that the user does not have write permission to, it's a bug.
--
Peter Eisentraut
http://developer.postgresql.org/~petere/
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 13:44 Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 13:44 UTC (permalink / raw)
To: Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> > It's not buggy. It's well documented behaviour,and per my linux
> > manpage for the file it's also OK per spec:
> >
> > The standard does not specify the directory that tmpfile()
> > will use. Glibc will try the path prefix P_tmpdir defined
> > in <stdio.h>, and if that fails the directory /tmp.
>
> The spec says
>
> The tmpfile() function shall create a temporary file and open a
> corresponding stream. The file shall be automatically
> deleted when all
> references to the file are closed. The file is opened as
> in fopen() for
> update (w+).
>
> If the implementation is such that it tries to create the
> file in a directory that the user does not have write
> permission to, it's a bug.
Well, you're never gonig to convince MS of that :-)
And either way, the runtime we're usnig now isn't especially current
(MSVC6), so even in the unlikely event that they did fix it, it wouldn't
help us. So we'll definitly need to fix it ourselves. Did the code I
sent the last time look reasonable?
//Magnus
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 14:00 Tom Lane <tgl@sss.pgh.pa.us>
parent: Magnus Hagander <mha@sollentuna.net>
1 sibling, 1 reply; 22+ messages in thread
From: Tom Lane @ 2006-04-20 14:00 UTC (permalink / raw)
To: Magnus Hagander <mha@sollentuna.net>; +Cc: Yoshiyuki Asaba <y-asaba@sraoss.co.jp>; pgsql-hackers
"Magnus Hagander" <mha@sollentuna.net> writes:
>> Win32's tmpfile() creates the file into root folder. But
>> non-administrator users can't create files into root folder.
In other words, tmpfile() doesn't work at all on Win32? Seems like
the appropriate place to be filing a bug report is at microsoft.com.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 14:03 Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 14:03 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Yoshiyuki Asaba <y-asaba@sraoss.co.jp>; pgsql-hackers
> >> Win32's tmpfile() creates the file into root folder. But
> >> non-administrator users can't create files into root folder.
>
> In other words, tmpfile() doesn't work at all on Win32?
> Seems like the appropriate place to be filing a bug report is
> at microsoft.com.
If works if you're an administrator or power user. Sure, we can file a
report, but if they bother to fix it, that will be for Visual Studio
2005 (if we're lucky, more likely for the next version after that). So
we can forget all about mingw then, since it uses an old version of the
runtime...
Though I doubt they'd fix it anyway, since their documentation alreadyu
tells you what to do to get around it.
//Magnus
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 14:10 Tom Lane <tgl@sss.pgh.pa.us>
parent: Peter Eisentraut <peter_e@gmx.net>
0 siblings, 0 replies; 22+ messages in thread
From: Tom Lane @ 2006-04-20 14:10 UTC (permalink / raw)
To: Peter Eisentraut <peter_e@gmx.net>; +Cc: Magnus Hagander <mha@sollentuna.net>; pgsql-hackers@postgresql.org, "Yoshiyuki Asaba" <y-asaba@sraoss.co.jp>
Peter Eisentraut <peter_e@gmx.net> writes:
> If the implementation is such that it tries to create the file in a directory
> that the user does not have write permission to, it's a bug.
Well, I think it would be a valid implementation on Unix to always try
to create the file in /tmp, which'd likely fail if someone had revoked
world write on /tmp --- but doing the latter is administrator error,
not a library fault.
OTOH, if / is *supposed* to be non world writable on Win32, then trying
to create temp files there indicates a seriously brain-damaged library.
It should be trying to create the file in a place where the user is
expected to have permission to do it.
Has anyone looked to see with tmpfile() actually does though? I'm a bit
surprised that it doesn't create stuff in the same directory as tmpnam().
I wonder if Magnus and Yoshiyuki are testing under different conditions.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 14:37 Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 22+ messages in thread
From: Yoshiyuki Asaba @ 2006-04-20 14:37 UTC (permalink / raw)
To: tgl@sss.pgh.pa.us; +Cc: mha@sollentuna.net; pgsql-hackers
From: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: [HACKERS] pg_dump -Ft failed on Windows XP
Date: Thu, 20 Apr 2006 10:00:48 -0400
> "Magnus Hagander" <mha@sollentuna.net> writes:
> >> Win32's tmpfile() creates the file into root folder. But
> >> non-administrator users can't create files into root folder.
>
> In other words, tmpfile() doesn't work at all on Win32? Seems like
> the appropriate place to be filing a bug report is at microsoft.com.
Yes. There is the description of Win32's tmpfile() at the following URL.
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/_crt_tmpfile.asp
--
Yoshiyuki Asaba
y-asaba@sraoss.co.jp
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 15:01 Magnus Hagander <mha@sollentuna.net>
0 siblings, 1 reply; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 15:01 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Peter Eisentraut <peter_e@gmx.net>; +Cc: pgsql-hackers; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> > If the implementation is such that it tries to create the file in a
> > directory that the user does not have write permission to,
> it's a bug.
>
> Well, I think it would be a valid implementation on Unix to
> always try to create the file in /tmp, which'd likely fail if
> someone had revoked world write on /tmp --- but doing the
> latter is administrator error, not a library fault.
>
> OTOH, if / is *supposed* to be non world writable on Win32,
> then trying to create temp files there indicates a seriously
> brain-damaged library.
> It should be trying to create the file in a place where the
> user is expected to have permission to do it.
Prior to Windows XP, users had write permissions in the root IIRC. They
definitly had in NT4, but I think they did in 2000 as well.
> Has anyone looked to see with tmpfile() actually does though?
> I'm a bit surprised that it doesn't create stuff in the same
> directory as tmpnam().
> I wonder if Magnus and Yoshiyuki are testing under different
> conditions.
I have repeated the problem with CVS head on XP SP2. It *does* create it
there (or rather, it tries to).
tmpnam() returns a file in the current dir per documentation, but I see
it generating one in the root instead.
tempnam() uses TMP environment variable.
tmpfile() and tmpnam() both use the same function to generate the
filename.
//Magnus
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 15:11 Tom Lane <tgl@sss.pgh.pa.us>
parent: Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 22+ messages in thread
From: Tom Lane @ 2006-04-20 15:11 UTC (permalink / raw)
To: Magnus Hagander <mha@sollentuna.net>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers@postgresql.org, "Yoshiyuki Asaba" <y-asaba@sraoss.co.jp>
"Magnus Hagander" <mha@sollentuna.net> writes:
> I have repeated the problem with CVS head on XP SP2. It *does* create it
> there (or rather, it tries to).
> tmpnam() returns a file in the current dir per documentation, but I see
> it generating one in the root instead.
> tempnam() uses TMP environment variable.
> tmpfile() and tmpnam() both use the same function to generate the
> filename.
Hm. I guess I concur with Peter's conclusion: the cleanest fix is to
put our own implementation of tmpfile() into src/port/.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 15:50 Magnus Hagander <mha@sollentuna.net>
0 siblings, 1 reply; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 15:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> > I have repeated the problem with CVS head on XP SP2. It
> *does* create
> > it there (or rather, it tries to).
>
> > tmpnam() returns a file in the current dir per documentation, but I
> > see it generating one in the root instead.
> > tempnam() uses TMP environment variable.
>
> > tmpfile() and tmpnam() both use the same function to generate the
> > filename.
>
> Hm. I guess I concur with Peter's conclusion: the cleanest
> fix is to put our own implementation of tmpfile() into src/port/.
Ok. Should be easy enough once the code is fine - can you comment on the
patch as sent, if the code itself looks right provided i wrap it up in a
function in port/?
//Magnus
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 16:07 Tom Lane <tgl@sss.pgh.pa.us>
parent: Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 22+ messages in thread
From: Tom Lane @ 2006-04-20 16:07 UTC (permalink / raw)
To: Magnus Hagander <mha@sollentuna.net>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers@postgresql.org, "Yoshiyuki Asaba" <y-asaba@sraoss.co.jp>
"Magnus Hagander" <mha@sollentuna.net> writes:
> Ok. Should be easy enough once the code is fine - can you comment on the
> patch as sent, if the code itself looks right provided i wrap it up in a
> function in port/?
Not sure if the error handling is adequate --- are there any cases
besides EEXIST that should loop?
A look at
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vclib/html/_crt__tempnam.2c_._wtemp...
suggests that tempnam() is also pretty fragile, esp. if you're passing
NULLs. Apparently it won't work at all if TMP isn't set?
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 17:35 Magnus Hagander <mha@sollentuna.net>
0 siblings, 1 reply; 22+ messages in thread
From: Magnus Hagander @ 2006-04-20 17:35 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> > Ok. Should be easy enough once the code is fine - can you
> comment on
> > the patch as sent, if the code itself looks right provided
> i wrap it
> > up in a function in port/?
>
> Not sure if the error handling is adequate --- are there any
> cases besides EEXIST that should loop?
Well, per docs I can get:
EACCES (will certainly fail again since we're still working the same
directory)
EEXIST (covered)
EINVAL = invalid oflag or pmode, which should never happen
EMFILE = too many open files (that would be the one, but I doubd it'd
help that much)
ENOENT = path ont found (would happen agai non retry)
> A look at
> http://msdn.microsoft.com/library/default.asp?url=/library/en-
> us/vclib/html/_crt__tempnam.2c_._wtempnam.2c_.tmpnam.2c_._wtmpnam.asp
>
> suggests that tempnam() is also pretty fragile, esp. if
> you're passing NULLs. Apparently it won't work at all if TMP
> isn't set?
I'm not *too* concerned about that, since TMP is normally set by the OS
itself. There's one set in the "system environment" (to c:\windows\temp
or whatrever) and then it's overridden by one set by the OS when it
loads a user profile.
Also to the point, what would you fall back to?
//Magnus
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-20 17:55 Tom Lane <tgl@sss.pgh.pa.us>
parent: Magnus Hagander <mha@sollentuna.net>
0 siblings, 0 replies; 22+ messages in thread
From: Tom Lane @ 2006-04-20 17:55 UTC (permalink / raw)
To: Magnus Hagander <mha@sollentuna.net>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers@postgresql.org, "Yoshiyuki Asaba" <y-asaba@sraoss.co.jp>
"Magnus Hagander" <mha@sollentuna.net> writes:
>> Apparently it won't work at all if TMP
>> isn't set?
> I'm not *too* concerned about that, since TMP is normally set by the OS
> itself. There's one set in the "system environment" (to c:\windows\temp
> or whatrever) and then it's overridden by one set by the OS when it
> loads a user profile.
OK, then maybe not having it would be equivalent to /tmp-not-writable
on Unix, ie, admin error.
> Also to the point, what would you fall back to?
Current directory maybe?
Actually, the thing that struck me about the man page was that first
it said current dir was a fallback, and then it said it wasn't. Is this
just typical Microsoft brain damage, or is there a reason to avoid using
current dir for this?
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-04-21 09:11 Zeugswetter Andreas DCP SD <ZeugswetterA@spardat.at>
0 siblings, 2 replies; 22+ messages in thread
From: Zeugswetter Andreas DCP SD @ 2006-04-21 09:11 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Magnus Hagander <mha@sollentuna.net>; +Cc: Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
> >> Apparently it won't work at all if TMP isn't set?
>
> > I'm not *too* concerned about that, since TMP is normally set by the
OS
> > itself. There's one set in the "system environment" (to
c:\windows\temp
> > or whatrever) and then it's overridden by one set by the OS when it
> > loads a user profile.
>
> OK, then maybe not having it would be equivalent to /tmp-not-writable
> on Unix, ie, admin error.
>
> > Also to the point, what would you fall back to?
>
> Current directory maybe?
It tries \ (tested on Win 2000), if the dir argument is NULL and TMP is
not set.
But TMP is usually set.
Attached is a working version not yet adapted to port/.
- memoryleak fixed
- use _tmpname and _fdopen not the compatibility tmpname and fdopen
(imho only cosmetic)
- EACCES fixed (Win2000 needs _S_IREAD | _S_IWRITE or fails with EACCES,
even as Admin)
- I suggest adding a prefix pg_temp_ (for leftover temp files after
crash,
the name I get is then usually pg_temp_2)
Andreas
Index: bin/pg_dump/pg_backup_tar.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.50
diff -c -r1.50 pg_backup_tar.c
*** bin/pg_dump/pg_backup_tar.c 12 Feb 2006 06:11:50 -0000 1.50
--- bin/pg_dump/pg_backup_tar.c 21 Apr 2006 09:22:00 -0000
***************
*** 362,368 ****
--- 362,388 ----
{
tm = calloc(1, sizeof(TAR_MEMBER));
+ #ifndef WIN32
tm->tmpFH = tmpfile();
+ #else
+ /* on win32, tmpfile() generates a filename in the root directory, which requires
+ * administrative permissions to write to. */
+ while (1)
+ {
+ char *tmpname;
+ int fd;
+
+ tmpname = _tempnam(NULL, "pg_temp_");
+ if (tmpname == NULL)
+ break;
+ fd = _open(tmpname, _O_RDWR | _O_CREAT | _O_EXCL | _O_BINARY | _O_TEMPORARY, _S_IREAD | _S_IWRITE);
+ free(tmpname);
+ if (fd == -1 && errno == EEXIST)
+ continue; /* Try again with a new name if file exists */
+ if (fd != -1)
+ tm->tmpFH = _fdopen(fd, "w+b");
+ break;
+ }
+ #endif
if (tm->tmpFH == NULL)
die_horribly(AH, modulename, "could not generate temporary file name: %s\n", strerror(errno));
Attachments:
[text/plain] pg_dump_tempfile.patch.txt (1.2K, ../../E1539E0ED7043848906A8FF995BDA579F318AB@m0143.s-mxs.net/2-pg_dump_tempfile.patch.txt)
download | inline:
Index: bin/pg_dump/pg_backup_tar.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.50
diff -c -r1.50 pg_backup_tar.c
*** bin/pg_dump/pg_backup_tar.c 12 Feb 2006 06:11:50 -0000 1.50
--- bin/pg_dump/pg_backup_tar.c 21 Apr 2006 09:22:00 -0000
***************
*** 362,368 ****
--- 362,388 ----
{
tm = calloc(1, sizeof(TAR_MEMBER));
+ #ifndef WIN32
tm->tmpFH = tmpfile();
+ #else
+ /* on win32, tmpfile() generates a filename in the root directory, which requires
+ * administrative permissions to write to. */
+ while (1)
+ {
+ char *tmpname;
+ int fd;
+
+ tmpname = _tempnam(NULL, "pg_temp_");
+ if (tmpname == NULL)
+ break;
+ fd = _open(tmpname, _O_RDWR | _O_CREAT | _O_EXCL | _O_BINARY | _O_TEMPORARY, _S_IREAD | _S_IWRITE);
+ free(tmpname);
+ if (fd == -1 && errno == EEXIST)
+ continue; /* Try again with a new name if file exists */
+ if (fd != -1)
+ tm->tmpFH = _fdopen(fd, "w+b");
+ break;
+ }
+ #endif
if (tm->tmpFH == NULL)
die_horribly(AH, modulename, "could not generate temporary file name: %s\n", strerror(errno));
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: pg_dump -Ft failed on Windows XP
@ 2006-06-16 21:36 Bruce Momjian <pgman@candle.pha.pa.us>
parent: Zeugswetter Andreas DCP SD <ZeugswetterA@spardat.at>
1 sibling, 0 replies; 22+ messages in thread
From: Bruce Momjian @ 2006-06-16 21:36 UTC (permalink / raw)
To: Zeugswetter Andreas DCP SD <ZeugswetterA@spardat.at>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Magnus Hagander <mha@sollentuna.net>; Peter Eisentraut <peter_e@gmx.net>; pgsql-hackers@postgresql.org, Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
Someday we can move this to /port, but for now, let's get it into CVS.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.
---------------------------------------------------------------------------
Zeugswetter Andreas DCP SD wrote:
>
> > >> Apparently it won't work at all if TMP isn't set?
> >
> > > I'm not *too* concerned about that, since TMP is normally set by the
> OS
> > > itself. There's one set in the "system environment" (to
> c:\windows\temp
> > > or whatrever) and then it's overridden by one set by the OS when it
> > > loads a user profile.
> >
> > OK, then maybe not having it would be equivalent to /tmp-not-writable
> > on Unix, ie, admin error.
> >
> > > Also to the point, what would you fall back to?
> >
> > Current directory maybe?
>
> It tries \ (tested on Win 2000), if the dir argument is NULL and TMP is
> not set.
> But TMP is usually set.
>
> Attached is a working version not yet adapted to port/.
> - memoryleak fixed
> - use _tmpname and _fdopen not the compatibility tmpname and fdopen
> (imho only cosmetic)
> - EACCES fixed (Win2000 needs _S_IREAD | _S_IWRITE or fails with EACCES,
> even as Admin)
> - I suggest adding a prefix pg_temp_ (for leftover temp files after
> crash,
> the name I get is then usually pg_temp_2)
>
> Andreas
Content-Description: pg_dump_tempfile.patch.txt
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: [HACKERS] pg_dump -Ft failed on Windows XP
@ 2006-06-27 01:18 Bruce Momjian <bruce@momjian.us>
parent: Zeugswetter Andreas DCP SD <ZeugswetterA@spardat.at>
1 sibling, 0 replies; 22+ messages in thread
From: Bruce Momjian @ 2006-06-27 01:18 UTC (permalink / raw)
To: Zeugswetter Andreas DCP SD <ZeugswetterA@spardat.at>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Magnus Hagander <mha@sollentuna.net>; Peter Eisentraut <peter_e@gmx.net>; Yoshiyuki Asaba <y-asaba@sraoss.co.jp>; PostgreSQL-patches <pgsql-patches@postgresql.org>
Modified patch attached and applied to HEAD and 8.1.X.
I restructured the loop exit, and used the symbols without the leading
underscores. I didn't see any Win32 underscore symbol usage in our
existing code.
Thanks.
-------------------------------------------------------------------------
Zeugswetter Andreas DCP SD wrote:
>
> > >> Apparently it won't work at all if TMP isn't set?
> >
> > > I'm not *too* concerned about that, since TMP is normally set by the
> OS
> > > itself. There's one set in the "system environment" (to
> c:\windows\temp
> > > or whatrever) and then it's overridden by one set by the OS when it
> > > loads a user profile.
> >
> > OK, then maybe not having it would be equivalent to /tmp-not-writable
> > on Unix, ie, admin error.
> >
> > > Also to the point, what would you fall back to?
> >
> > Current directory maybe?
>
> It tries \ (tested on Win 2000), if the dir argument is NULL and TMP is
> not set.
> But TMP is usually set.
>
> Attached is a working version not yet adapted to port/.
> - memoryleak fixed
> - use _tmpname and _fdopen not the compatibility tmpname and fdopen
> (imho only cosmetic)
> - EACCES fixed (Win2000 needs _S_IREAD | _S_IWRITE or fails with EACCES,
> even as Admin)
> - I suggest adding a prefix pg_temp_ (for leftover temp files after
> crash,
> the name I get is then usually pg_temp_2)
>
> Andreas
Content-Description: pg_dump_tempfile.patch.txt
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachments:
[text/x-diff] /rtmp/diff (1.3K, ../../200606270118.k5R1IiW02062@momjian.us/2-%2Frtmp%2Fdiff)
download | inline diff:
Index: src/bin/pg_dump/pg_backup_tar.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_tar.c,v
retrieving revision 1.52
diff -c -c -r1.52 pg_backup_tar.c
*** src/bin/pg_dump/pg_backup_tar.c 7 Jun 2006 22:24:44 -0000 1.52
--- src/bin/pg_dump/pg_backup_tar.c 27 Jun 2006 00:32:06 -0000
***************
*** 359,365 ****
--- 359,393 ----
{
tm = calloc(1, sizeof(TAR_MEMBER));
+ #ifndef WIN32
tm->tmpFH = tmpfile();
+ #else
+ /*
+ * On WIN32, tmpfile() generates a filename in the root directory,
+ * which requires administrative permissions on certain systems.
+ * Loop until we find a unique file name we can create.
+ */
+ while (1)
+ {
+ char *name;
+ int fd;
+
+ name = _tempnam(NULL, "pg_temp_");
+ if (name == NULL)
+ break;
+ fd = open(name, O_RDWR | O_CREAT | O_EXCL | O_BINARY |
+ O_TEMPORARY, S_IREAD | S_IWRITE);
+ free(name);
+
+ if (fd != -1) /* created a file */
+ {
+ tm->tmpFH = fdopen(fd, "w+b");
+ break;
+ }
+ else if (errno != EEXIST) /* failure other than file exists */
+ break;
+ }
+ #endif
if (tm->tmpFH == NULL)
die_horribly(AH, modulename, "could not generate temporary file name: %s\n", strerror(errno));
^ permalink raw reply [nested|flat] 22+ messages in thread
end of thread, other threads:[~2006-06-27 01:18 UTC | newest]
Thread overview: 22+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2006-04-20 04:06 pg_dump -Ft failed on Windows XP Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
2006-04-20 08:47 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 10:45 ` Peter Eisentraut <peter_e@gmx.net>
2006-04-20 14:00 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-04-20 14:37 ` Yoshiyuki Asaba <y-asaba@sraoss.co.jp>
2006-04-20 11:03 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 11:19 ` Peter Eisentraut <peter_e@gmx.net>
2006-04-20 11:19 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 11:21 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 11:53 ` Peter Eisentraut <peter_e@gmx.net>
2006-04-20 14:10 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-04-20 13:44 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 14:03 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 15:01 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 15:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-04-20 15:50 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 16:07 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-04-20 17:35 Re: pg_dump -Ft failed on Windows XP Magnus Hagander <mha@sollentuna.net>
2006-04-20 17:55 ` Tom Lane <tgl@sss.pgh.pa.us>
2006-04-21 09:11 Re: pg_dump -Ft failed on Windows XP Zeugswetter Andreas DCP SD <ZeugswetterA@spardat.at>
2006-06-16 21:36 ` Bruce Momjian <pgman@candle.pha.pa.us>
2006-06-27 01:18 ` Bruce Momjian <bruce@momjian.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox