This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, libfortran] PR46267 strerror() might not be thread-safe, take 2
On Fri, Jan 21, 2011 at 19:36, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Jan 21, 2011 at 07:24:56PM +0200, Janne Blomqvist wrote:
>> @@ -141,6 +141,38 @@ gfc_xtoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
>> Â Âreturn p;
>> Â}
>>
>> +
>> +/* Hopefully thread-safe wrapper for a strerror() style function. Â*/
>> +
>> +char *
>> +gf_strerror (int errnum,
>> +#ifdef HAVE_STRERROR_R
>> + Â Â Â Â Âchar * buf, size_t buflen)
>> +{
>> + Â/* TODO: How to prevent the compiler warning due to strerror_r of
>> + Â Â the untaken branch having the wrong return type? Â*/
>> + Âif (__builtin_classify_type (strerror_r (0, buf, 0)) == 5)
>> + Â Â{
>> + Â Â Â/* GNU strerror_r() Â*/
>> + Â Â Âreturn strerror_r (errnum, buf, buflen);
>> + Â Â}
>> + Âelse
>> + Â Â{
>> + Â Â Â/* POSIX strerror_r () Â*/
>> + Â Â Âstrerror_r (errnum, buf, buflen);
>> + Â Â Âreturn buf;
>> + Â Â}
>> +#else
>> + Â Â Â Â Â Â char * buf __attribute__((unused)),
>> + Â Â Â Â Âsize_t buflen __attribute__((unused)))
>> +{
>> + Â/* strerror () is not necessarily thread-safe, but should at least
>> + Â Â be available everywhere. Â*/
>> + Âreturn strerror (errnum);
>> +#endif
>> +}
>
> The #ifdef inside of the parameters ending inside function
> looks very ugly, you can use unused attribute unconditionally
> and start #ifdef only within function body.
Fair enough; updated patch attached. Otherwise no changes. Ok for trunk?
--
Janne Blomqvist
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index 7b28f12..4f137e4 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -249,7 +249,7 @@ AC_CHECK_FUNCS(chdir strerror getlogin gethostname kill link symlink perror)
AC_CHECK_FUNCS(sleep time ttyname signal alarm ctime clock access fork execl)
AC_CHECK_FUNCS(wait setmode execvp pipe dup2 close fdopen strcasestr getrlimit)
AC_CHECK_FUNCS(gettimeofday stat fstat lstat getpwuid vsnprintf dup getcwd)
-AC_CHECK_FUNCS(localtime_r gmtime_r)
+AC_CHECK_FUNCS(localtime_r gmtime_r strerror_r)
# Check for glibc backtrace functions
AC_CHECK_FUNCS(backtrace backtrace_symbols)
diff --git a/libgfortran/intrinsics/gerror.c b/libgfortran/intrinsics/gerror.c
index ccb5c3e..6feadc9 100644
--- a/libgfortran/intrinsics/gerror.c
+++ b/libgfortran/intrinsics/gerror.c
@@ -43,16 +43,17 @@ PREFIX(gerror) (char * msg, gfc_charlen_type msg_len)
int p_len;
char *p;
- memset (msg, ' ', msg_len); /* Blank the string. */
-
- p = strerror (errno);
- if (p == NULL)
- return;
-
+ p = gf_strerror (errno, msg, msg_len);
p_len = strlen (p);
- if (msg_len < p_len)
- memcpy (msg, p, msg_len);
- else
- memcpy (msg, p, p_len);
+ /* The returned pointer p might or might not be the same as the msg
+ argument. */
+ if (p != msg)
+ {
+ if (msg_len < p_len)
+ p_len = msg_len;
+ memcpy (msg, p, p_len);
+ }
+ if (msg_len > p_len)
+ memset (&msg[p_len], ' ', msg_len - p_len);
}
#endif
diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c
index fa64e20..950b7a2 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -256,16 +256,6 @@ flush_if_preconnected (stream * s)
}
-/* get_oserror()-- Get the most recent operating system error. For
- * unix, this is errno. */
-
-const char *
-get_oserror (void)
-{
- return strerror (errno);
-}
-
-
/********************************************************************
Raw I/O functions (read, write, seek, tell, truncate, close).
diff --git a/libgfortran/libgfortran.h b/libgfortran/libgfortran.h
index ac86492..c9d3f37 100644
--- a/libgfortran/libgfortran.h
+++ b/libgfortran/libgfortran.h
@@ -1,5 +1,6 @@
/* Common declarations for all of libgfortran.
- Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010
+ Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
+ 2011
Free Software Foundation, Inc.
Contributed by Paul Brook <paul@nowt.org>, and
Andy Vaught <andy@xena.eas.asu.edu>
@@ -738,9 +739,6 @@ extern void internal_error (st_parameter_common *, const char *)
__attribute__ ((noreturn));
internal_proto(internal_error);
-extern const char *get_oserror (void);
-internal_proto(get_oserror);
-
extern const char *translate_error (int);
internal_proto(translate_error);
@@ -756,6 +754,9 @@ internal_proto(notify_std);
extern notification notification_std(int);
internal_proto(notification_std);
+extern char *gf_strerror (int, char *, size_t);
+internal_proto(gf_strerror);
+
/* fpu.c */
extern void set_fpu (void);
diff --git a/libgfortran/runtime/error.c b/libgfortran/runtime/error.c
index 1baf9d3..06c144a 100644
--- a/libgfortran/runtime/error.c
+++ b/libgfortran/runtime/error.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2002, 2003, 2005, 2006, 2007, 2009, 2010
+/* Copyright (C) 2002, 2003, 2005, 2006, 2007, 2009, 2010, 2011
Free Software Foundation, Inc.
Contributed by Andy Vaught
@@ -141,6 +141,36 @@ gfc_xtoa (GFC_UINTEGER_LARGEST n, char *buffer, size_t len)
return p;
}
+
+/* Hopefully thread-safe wrapper for a strerror_r() style function. */
+
+char *
+gf_strerror (int errnum,
+ char * buf __attribute__((unused)),
+ size_t buflen __attribute__((unused)))
+{
+#ifdef HAVE_STRERROR_R
+ /* TODO: How to prevent the compiler warning due to strerror_r of
+ the untaken branch having the wrong return type? */
+ if (__builtin_classify_type (strerror_r (0, buf, 0)) == 5)
+ {
+ /* GNU strerror_r() */
+ return strerror_r (errnum, buf, buflen);
+ }
+ else
+ {
+ /* POSIX strerror_r () */
+ strerror_r (errnum, buf, buflen);
+ return buf;
+ }
+#else
+ /* strerror () is not necessarily thread-safe, but should at least
+ be available everywhere. */
+ return strerror (errnum);
+#endif
+}
+
+
/* show_locus()-- Print a line number and filename describing where
* something went wrong */
@@ -192,6 +222,8 @@ recursion_check (void)
}
+#define STRERR_MAXSZ 256
+
/* os_error()-- Operating system error. We get a message from the
* operating system, show it and leave. Some operating system errors
* are caught and processed by the library. If not, we come here. */
@@ -199,8 +231,10 @@ recursion_check (void)
void
os_error (const char *message)
{
+ char errmsg[STRERR_MAXSZ];
recursion_check ();
- st_printf ("Operating system error: %s\n%s\n", get_oserror (), message);
+ st_printf ("Operating system error: %s\n%s\n",
+ gf_strerror (errno, errmsg, STRERR_MAXSZ), message);
sys_exit (1);
}
iexport(os_error);
@@ -389,6 +423,7 @@ translate_error (int code)
void
generate_error (st_parameter_common *cmp, int family, const char *message)
{
+ char errmsg[STRERR_MAXSZ];
/* If there was a previous error, don't mask it with another
error message, EOF or EOR condition. */
@@ -402,7 +437,8 @@ generate_error (st_parameter_common *cmp, int family, const char *message)
if (message == NULL)
message =
- (family == LIBERROR_OS) ? get_oserror () : translate_error (family);
+ (family == LIBERROR_OS) ? gf_strerror (errno, errmsg, STRERR_MAXSZ) :
+ translate_error (family);
if (cmp->flags & IOPARM_HAS_IOMSG)
cf_strcpy (cmp->iomsg, cmp->iomsg_len, message);