Re: [BUGS] BUG #6166: configure from source fails with 'This platform is not thread-safe.' but was actually /tmp perms

2011-08-20 Thread Tom Lane
Bruce Momjian  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.

regards, tom lane

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #6166: configure from source fails with 'This platform is not thread-safe.' but was actually /tmp perms

2011-08-20 Thread Bruce Momjian
Tom Lane wrote:
> Alex Soto  writes:
> > Here's the section in the config.log in case it makes a difference
> 
> > configure:28808: ./conftest
> > Could not create file in /tmp or
> > Could not generate failure for create file in /tmp **
> > exiting
> > configure:28812: $? = 1
> > configure: program exited with status 1
> 
> [ greps... ]  Oh, that error is coming from src/test/thread/thread_test.c.
> 
> I wonder why we have that trying to write to /tmp at all, when all the
> other transient junk generated by configure is in the current directory.
> Bruce, do you have a good reason for doing it that way?

I have modified the code to use the current directory instead of /tmp. 
I also cleaned up the #if defines and added some C comments.

> (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()?

-- 
  Bruce Momjian  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 6a81829..2271ba6
*** a/src/test/thread/thread_test.c
--- b/src/test/thread/thread_test.c
*** typedef char bool;
*** 52,92 
  #include 
  #endif
  
- /**
-  * Windows Hacks
-  */
- 
  #ifdef WIN32
  #define MAXHOSTNAMELEN 63
  #include 
- 
- int			mkstemp(char *template);
- 
- int
- mkstemp(char *template)
- {
- 	FILE	   *foo;
- 
- 	mktemp(template);
- 	foo = fopen(template, "rw");
- 	if (!foo)
- 		return -1;
- 	else
- 		return (int) foo;
- }
  #endif
  
- /**
-  * End Windows Hacks
-  */
- 
  
  /* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */
  #include 
  int			sigwait(const sigset_t *set, int *sig);
  
  
! #if !defined(ENABLE_THREAD_SAFETY) && !defined(IN_CONFIGURE) && !(defined(WIN32))
  int
  main(int argc, char *argv[])
  {
--- 52,69 
  #include 
  #endif
  
  #ifdef WIN32
  #define MAXHOSTNAMELEN 63
  #include 
  #endif
  
  
  /* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */
  #include 
  int			sigwait(const sigset_t *set, int *sig);
  
  
! #if !defined(ENABLE_THREAD_SAFETY) && !defined(IN_CONFIGURE) && !defined(WIN32)
  int
  main(int argc, char *argv[])
  {
*** main(int argc, char *argv[])
*** 99,118 
  /* This must be down here because this is the code that uses threads. */
  #include 
  
  static void func_call_1(void);
  static void func_call_2(void);
  
- #ifdef WIN32
- #define		TEMP_FILENAME_1 "thread_test.1.XX"
- #define		TEMP_FILENAME_2 "thread_test.2.XX"
- #else
- #define		TEMP_FILENAME_1 "/tmp/thread_test.1.XX"
- #define		TEMP_FILENAME_2 "/tmp/thread_test.2.XX"
- #endif
- 
- static char *temp_filename_1;
- static char *temp_filename_2;
- 
  static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
  
  static volatile int thread1_done = 0;
--- 76,87 
  /* This must be down here because this is the code that uses threads. */
  #include 
  
+ #define		TEMP_FILENAME_1 "thread_test.1"
+ #define		TEMP_FILENAME_2 "thread_test.2"
+ 
  static void func_call_1(void);
  static void func_call_2(void);
  
  static pthread_mutex_t init_mutex = PTHREAD_MUTEX_INITIALIZER;
  
  static volatile int thread1_done = 0;
*** static char *strerror_p2;
*** 127,139 
  static bool strerror_threadsafe = false;
  #endif
  
! #ifndef WIN32
! #ifndef HAVE_GETPWUID_R
  static struct passwd *passwd_p1;
  static struct passwd *passwd_p2;
  static bool getpwuid_threadsafe = false;
  #endif
- #endif
  
  #if !defined(HAVE_GETADDRINFO) && !defined(HAVE_GETHOSTBYNAME_R)
  static struct hostent *hostent_p1;
--- 96,106 
  static bool strerror_threadsafe = false;
  #endif
  
! #if !defined(WIN32) && !d

Re: [BUGS] BUG #6166: configure from source fails with 'This platform is not thread-safe.' but was actually /tmp perms

2011-08-20 Thread Bruce Momjian
Tom Lane wrote:
> Alex Soto  writes:
> > Here's the section in the config.log in case it makes a difference
> 
> > configure:28808: ./conftest
> > Could not create file in /tmp or
> > Could not generate failure for create file in /tmp **
> > exiting
> > configure:28812: $? = 1
> > configure: program exited with status 1
> 
> [ greps... ]  Oh, that error is coming from src/test/thread/thread_test.c.
> 
> I wonder why we have that trying to write to /tmp at all, when all the
> other transient junk generated by configure is in the current directory.
> Bruce, do you have a good reason for doing it that way?

No idea --- using the current directory is probably best.

> (The error message seems to be suffering from a bad case of copy-and-
> paste-itis, too.)

OK, let me see if I can fix this.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs