Tom Lane wrote:
> Bruce Momjian <br...@momjian.us> writes:
> > Tom Lane wrote:
> >> (The error message seems to be suffering from a bad case of copy-and-
> >> paste-itis, too.)
>
> > Actually, it is accurate. The code is:
>
> > #ifdef WIN32
> > h1 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL,
> > OPEN_ALWAYS, 0, NULL);
> > h2 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL,
> > CREATE_NEW, 0, NULL);
> > if (h1 == INVALID_HANDLE_VALUE || GetLastError() !=
> > ERROR_FILE_EXISTS)
> > #else
> > if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0 ||
> > open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
> > #endif
> > {
> > fprintf(stderr, "Could not create file in current directory
> > or\n");
> > fprintf(stderr, "could not generate failure for create file in
> > current directory **\nexiting\n");
> > exit(1);
> > }
>
> > This code generates an errno == EEXIST in one thread, while another
> > thread generates errno == ENOENT, and this is how we test for errno
> > being thread-safe. If you have a cleaner way to do this, please let me
> > know. mkdir()?
>
> The problem with that is you're trying to make one error message serve
> for two extremely different failure conditions. I think this should be
> coded more like
>
> if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0)
> {
> report suitable failure message;
> exit(1);
> }
> if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
> {
> report suitable failure message;
> exit(1);
> }
>
> You would probably find that the messages could be a lot more clear and
> specific if they were done like that. Also, a file-related error
> message that doesn't provide the filename nor strerror(errno) is pretty
> much wrong on its face, in my book.
OK, I split apart the two operations with the attached, applied patch.
There must be an easier way to generate two unique errno values in a
platform-indepentent way, but I can't find it. I did simplify the
second ENOENT generation by just doing unlink twice.
I can't just set errno to a different value, can I?
I am not able to use strerror because I don't know if that is
thread-safe yet. I do have a C comment about it:
/*
* strerror() might not be thread-safe, and we already spawned thread
* 1 that uses it, so avoid using it.
*/
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/test/thread/thread_test.c b/src/test/thread/thread_test.c
new file mode 100644
index 2271ba6..6fef840
*** a/src/test/thread/thread_test.c
--- b/src/test/thread/thread_test.c
*************** main(int argc, char *argv[])
*** 263,268 ****
--- 263,271 ----
}
}
+ /*
+ * func_call_1
+ */
static void
func_call_1(void)
{
*************** func_call_1(void)
*** 272,294 ****
void *p;
#endif
#ifdef WIN32
! HANDLE h1, h2;
#endif
unlink(TEMP_FILENAME_1);
/* create, then try to fail on exclusive create open */
#ifdef WIN32
! h1 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, OPEN_ALWAYS, 0, NULL);
! h2 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL);
! if (h1 == INVALID_HANDLE_VALUE || GetLastError() != ERROR_FILE_EXISTS)
#else
! if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600) < 0 ||
! open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
#endif
{
! fprintf(stderr, "Could not create file in current directory or\n");
! fprintf(stderr, "could not generate failure for create file in current directory **\nexiting\n");
exit(1);
}
--- 275,312 ----
void *p;
#endif
#ifdef WIN32
! HANDLE h1;
! #else
! int fd;
#endif
unlink(TEMP_FILENAME_1);
+ /* Set errno = EEXIST */
+
/* create, then try to fail on exclusive create open */
#ifdef WIN32
! if ((h1 = CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, OPEN_ALWAYS, 0, NULL)) ==
! INVALID_HANDLE_VALUE)
#else
! if ((fd = open(TEMP_FILENAME_1, O_RDWR | O_CREAT, 0600)) < 0)
#endif
{
! fprintf(stderr, "Could not create file %s in current directory\n",
! TEMP_FILENAME_1);
! exit(1);
! }
!
! #ifdef WIN32
! if (CreateFile(TEMP_FILENAME_1, GENERIC_WRITE, 0, NULL, CREATE_NEW, 0, NULL)
! != INVALID_HANDLE_VALUE || GetLastError() != ERROR_FILE_EXISTS)
! #else
! if (open(TEMP_FILENAME_1, O_RDWR | O_CREAT | O_EXCL, 0600) >= 0)
! #endif
! {
! fprintf(stderr,
! "Could not generate failure for exclusive file create of %s in current directory **\nexiting\n",
! TEMP_FILENAME_1);
exit(1);
}
*************** func_call_1(void)
*** 315,320 ****
--- 333,343 ----
exit(1);
}
+ #ifdef WIN32
+ CloseHandle(h1);
+ #else
+ close(fd);
+ #endif
unlink(TEMP_FILENAME_1);
#ifndef HAVE_STRERROR_R
*************** func_call_1(void)
*** 352,357 ****
--- 375,383 ----
}
+ /*
+ * func_call_2
+ */
static void
func_call_2(void)
{
*************** func_call_2(void)
*** 363,377 ****
unlink(TEMP_FILENAME_2);
! /* open non-existant file */
! #ifdef WIN32
! CreateFile(TEMP_FILENAME_2, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL);
! if (GetLastError() != ERROR_FILE_NOT_FOUND)
! #else
! if (open(TEMP_FILENAME_2, O_RDONLY, 0600) >= 0)
! #endif
{
! fprintf(stderr, "Read-only open succeeded without create **\nexiting\n");
exit(1);
}
--- 389,402 ----
unlink(TEMP_FILENAME_2);
! /* Set errno = ENOENT */
!
! /* This will fail, but we can't check errno yet */
! if (unlink(TEMP_FILENAME_2) != -1)
{
! fprintf(stderr,
! "Could not generate failure for unlink of %s in current directory **\nexiting\n",
! TEMP_FILENAME_2);
exit(1);
}
*************** func_call_2(void)
*** 394,405 ****
#else
fprintf(stderr, "errno not thread-safe **\nexiting\n");
#endif
- unlink(TEMP_FILENAME_2);
exit(1);
}
- unlink(TEMP_FILENAME_2);
-
#ifndef HAVE_STRERROR_R
/*
* If strerror() uses sys_errlist, the pointer might change for different
--- 419,427 ----
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
