Avoid using the non-thread-safe strerror() function

https://bugzilla.redhat.com/show_bug.cgi?id=985481

13 files changed, 160 insertions, 42 deletions

diff --git a/common/compat.c b/common/compat.c
index ed523c6..7df9954 100644
--- a/common/compat.c
+++ b/common/compat.c
@@ -797,3 +797,19 @@ getauxval (unsigned long type)
 }
 
 #endif /* HAVE_GETAUXVAL */
+
+#ifndef HAVE_STRERROR_R
+
+int
+strerror_r (int errnum,
+            char *buf,
+            size_t buflen)
+{
+#ifdef OS_WIN32
+	return strerror_s (buf, buflen, errnum);
+#else
+	#error no strerror_r implementation
+#endif
+}
+
+#endif /* HAVE_STRERROR_R */
diff --git a/common/compat.h b/common/compat.h
index fae1b7d..a1bccb8 100644
--- a/common/compat.h
+++ b/common/compat.h
@@ -312,3 +312,11 @@ unsigned long     getauxval (unsigned long type);
 #endif /* !HAVE_GETAUXVAL */
 
 #endif /* __COMPAT_H__ */
+
+#ifndef HAVE_STRERROR_R
+
+int         strerror_r      (int errnum,
+                             char *buf,
+                             size_t buflen);
+
+#endif /* HAVE_STRERROR_R */
diff --git a/common/message.c b/common/message.c
index 4358e46..35f2764 100644
--- a/common/message.c
+++ b/common/message.c
@@ -92,6 +92,32 @@ p11_message_store (const char* msg,
 }
 
 void
+p11_message_err (int errnum,
+                 const char* msg,
+                 ...)
+{
+	char buffer[P11_MESSAGE_MAX];
+	char strerr[P11_MESSAGE_MAX];
+	va_list va;
+	size_t length;
+
+	va_start (va, msg);
+	length = vsnprintf (buffer, P11_MESSAGE_MAX - 1, msg, va);
+	va_end (va);
+
+	/* Was it truncated? */
+	if (length > P11_MESSAGE_MAX - 1)
+		length = P11_MESSAGE_MAX - 1;
+	buffer[length] = 0;
+
+	strncpy (strerr, "Unknown error", sizeof (strerr));
+	strerror_r (errnum, strerr, sizeof (strerr));
+	strerr[P11_MESSAGE_MAX - 1] = 0;
+
+	p11_message ("%s: %s", buffer, strerr);
+}
+
+void
 p11_message (const char* msg,
              ...)
 {
diff --git a/common/message.h b/common/message.h
index 60a7f81..3fe86df 100644
--- a/common/message.h
+++ b/common/message.h
@@ -48,6 +48,10 @@ extern char * (* p11_message_storage)      (void);
 void          p11_message                  (const char* msg,
                                             ...) GNUC_PRINTF (1, 2);
 
+void          p11_message_err              (int errnum,
+                                            const char* msg,
+                                            ...) GNUC_PRINTF (2, 3);
+
 void          p11_message_store            (const char* msg,
                                             size_t length);
 
diff --git a/common/path.c b/common/path.c
index 398c51e..fdde2ce 100644
--- a/common/path.c
+++ b/common/path.c
@@ -144,8 +144,7 @@ expand_homedir (const char *remainder)
 		}
 		if (ret < 0) {
 			error = errno;
-			p11_message ("couldn't lookup home directory for user %d: %s",
-				     getuid (), strerror (errno));
+			p11_message_err (errno, "couldn't lookup home directory for user %d", getuid ());
 			errno = error;
 			return NULL;
 		}
diff --git a/common/tests/Makefile.am b/common/tests/Makefile.am
index c230f6e..36292cd 100644
--- a/common/tests/Makefile.am
+++ b/common/tests/Makefile.am
@@ -23,6 +23,7 @@ CHECK_PROGS = \
 	test-buffer \
 	test-url \
 	test-path \
+	test-message \
 	$(NULL)
 
 noinst_PROGRAMS = \
diff --git a/common/tests/test-message.c b/common/tests/test-message.c
new file mode 100644
index 0000000..f1897d2
--- /dev/null
+++ b/common/tests/test-message.c
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2013 Red Hat Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *     * Redistributions of source code must retain the above
+ *       copyright notice, this list of conditions and the
+ *       following disclaimer.
+ *     * Redistributions in binary form must reproduce the
+ *       above copyright notice, this list of conditions and
+ *       the following disclaimer in the documentation and/or
+ *       other materials provided with the distribution.
+ *     * The names of contributors to this software may not be
+ *       used to endorse or promote products derived from this
+ *       software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ * Author: Stef Walter <stefw@redhat.com>
+ */
+
+#include "config.h"
+#include "CuTest.h"
+
+#include "message.h"
+
+#include <assert.h>
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void
+test_with_err (CuTest *tc)
+{
+	const char *last;
+	char *expected;
+
+	errno = E2BIG;
+	p11_message_err (ENOENT, "Details: %s", "value");
+	last = p11_message_last ();
+
+	if (asprintf (&expected, "Details: value: %s", strerror (ENOENT)) < 0)
+		assert (false);
+	CuAssertStrEquals (tc, expected, last);
+	free (expected);
+}
+
+int
+main (void)
+{
+	CuString *output = CuStringNew ();
+	CuSuite* suite = CuSuiteNew ();
+	int ret;
+
+	putenv ("P11_KIT_STRICT=1");
+
+	SUITE_ADD_TEST (suite, test_with_err);
+
+	CuSuiteRun (suite);
+	CuSuiteSummary (suite, output);
+	CuSuiteDetails (suite, output);
+	printf ("%s\n", output->buffer);
+	ret = suite->failCount;
+	CuSuiteDelete (suite);
+	CuStringDelete (output);
+
+	return ret;
+}
diff --git a/configure.ac b/configure.ac
index 1b7cdac..33c1e18 100644
--- a/configure.ac
+++ b/configure.ac
@@ -77,7 +77,7 @@ if test "$os_unix" = "yes"; then
 	AC_CHECK_MEMBERS([struct dirent.d_type],,,[#include <dirent.h>])
 	AC_CHECK_FUNCS([getprogname getexecname basename mkstemp mkdtemp])
 	AC_CHECK_FUNCS([getauxval issetugid getresuid])
-	AC_CHECK_FUNCS([strnstr memdup strndup])
+	AC_CHECK_FUNCS([strnstr memdup strndup strerror_r])
 	AC_CHECK_FUNCS([asprintf vasprintf vsnprintf])
 	AC_CHECK_FUNCS([timegm])
 
diff --git a/p11-kit/conf.c b/p11-kit/conf.c
index ed871ec..481ce26 100644
--- a/p11-kit/conf.c
+++ b/p11-kit/conf.c
@@ -118,8 +118,7 @@ _p11_conf_parse_file (const char* filename, int flags)
 			p11_debug ("config file is inaccessible");
 
 		} else {
-			p11_message ("couldn't open config file: %s: %s", filename,
-			             strerror (error));
+			p11_message_err (error, "couldn't open config file: %s", filename);
 			errno = error;
 			return NULL;
 		}
@@ -400,8 +399,7 @@ load_configs_from_directory (const char *directory,
 			p11_debug ("couldn't list inacessible module configs");
 			return true;
 		}
-		p11_message ("couldn't list directory: %s: %s", directory,
-		             strerror (error));
+		p11_message_err (error, "couldn't list directory: %s", directory);
 		errno = error;
 		return false;
 	}
diff --git a/tools/save.c b/tools/save.c
index f1605a3..8520a0c 100644
--- a/tools/save.c
+++ b/tools/save.c
@@ -108,8 +108,7 @@ p11_save_open_file (const char *path,
 
 	fd = mkstemp (temp);
 	if (fd < 0) {
-		p11_message ("couldn't create file: %s: %s",
-		             path, strerror (errno));
+		p11_message_err (errno, "couldn't create file: %s", path);
 		free (temp);
 		return NULL;
 	}
@@ -149,8 +148,7 @@ p11_save_write (p11_save_file *file,
 		if (res <= 0) {
 			if (errno == EAGAIN && errno == EINTR)
 				continue;
-			p11_message ("couldn't write to file: %s: %s",
-			             file->temp, strerror (errno));
+			p11_message_err (errno, "couldn't write to file: %s", file->temp);
 			return false;
 		} else {
 			written += res;
@@ -185,23 +183,20 @@ p11_save_finish_file (p11_save_file *file,
 	}
 
 	if (close (file->fd) < 0) {
-		p11_message ("couldn't write file: %s: %s",
-		             file->temp, strerror (errno));
+		p11_message_err (errno, "couldn't write file: %s", file->temp);
 		ret = false;
 
 #ifdef OS_UNIX
 	/* Set the mode of the file, readable by everyone, but not writable */
 	} else if (chmod (file->temp, S_IRUSR | S_IRGRP | S_IROTH) < 0) {
-		p11_message ("couldn't set file permissions: %s: %s",
-		             file->temp, strerror (errno));
+		p11_message_err (errno, "couldn't set file permissions: %s", file->temp);
 		close (file->fd);
 		ret = false;
 
 	/* Atomically rename the tempfile over the filename */
 	} else if (file->flags & P11_SAVE_OVERWRITE) {
 		if (rename (file->temp, file->path) < 0) {
-			p11_message ("couldn't complete writing file: %s: %s",
-			             file->path, strerror (errno));
+			p11_message_err (errno, "couldn't complete writing file: %s", file->path);
 			ret = false;
 		} else {
 			unlink (file->temp);
@@ -210,8 +205,7 @@ p11_save_finish_file (p11_save_file *file,
 	/* When not overwriting, link will fail if filename exists. */
 	} else {
 		if (link (file->temp, file->path) < 0) {
-			p11_message ("couldn't complete writing of file: %s: %s",
-			             file->path, strerror (errno));
+			p11_message_err (errno, "couldn't complete writing of file: %s", file->path);
 			ret = false;
 		}
 		unlink (file->temp);
@@ -222,16 +216,14 @@ p11_save_finish_file (p11_save_file *file,
 	} else {
 		if (file->flags & P11_SAVE_OVERWRITE) {
 			if (unlink (file->path) < 0 && errno != ENOENT) {
-				p11_message ("couldn't remove original file: %s: %s",
-				             file->path, strerror (errno));
+				p11_message_err (errno, "couldn't remove original file: %s", file->path);
 				ret = false;
 			}
 		}
 
 		if (ret == true) {
 			if (rename (file->temp, file->path) < 0) {
-				p11_message ("couldn't complete writing file: %s: %s",
-				             file->path, strerror (errno));
+				p11_message_err (errno, "couldn't complete writing file: %s", file->path);
 				ret = false;
 			}
 		}
@@ -264,7 +256,7 @@ p11_save_open_directory (const char *path,
 #endif
 		/* Some random error, report it */
 		if (errno != EEXIST) {
-			p11_message ("couldn't create directory: %s: %s", path, strerror (errno));
+			p11_message_err (errno, "couldn't create directory: %s", path);
 
 		/* The directory exists and we're not overwriting */
 		} else if (!(flags & P11_SAVE_OVERWRITE)) {
@@ -280,8 +272,7 @@ p11_save_open_directory (const char *path,
 		if (stat (path, &sb) >= 0) {
 			if ((sb.st_mode & S_IRWXU) != S_IRWXU &&
 			    chmod (path, S_IRWXU | sb.st_mode) < 0) {
-				p11_message ("couldn't make directory writable: %s: %s",
-				             path, strerror (errno));
+				p11_message_err (errno, "couldn't make directory writable: %s", path);
 				return NULL;
 			}
 		}
@@ -418,8 +409,7 @@ p11_save_symlink_in (p11_save_dir *dir,
 	unlink (path);
 
 	if (symlink (destination, path) < 0) {
-		p11_message ("couldn't create symlink: %s: %s",
-		             path, strerror (errno));
+		p11_message_err (errno, "couldn't create symlink: %s", path);
 		ret = false;
 	} else {
 		if (!p11_dict_set (dir->cache, name, name))
@@ -451,8 +441,7 @@ cleanup_directory (const char *directory,
 	/* First we load all the modules */
 	dir = opendir (directory);
 	if (!dir) {
-		p11_message ("couldn't list directory: %s: %s",
-		             directory, strerror (errno));
+		p11_message_err (errno, "couldn't list directory: %s", directory);
 		return false;
 	}
 
@@ -493,8 +482,7 @@ cleanup_directory (const char *directory,
 	p11_dict_iterate (remove, &iter);
 	while (p11_dict_next (&iter, (void **)&path, NULL)) {
 		if (unlink (path) < 0 && errno != ENOENT) {
-			p11_message ("couldn't remove file: %s: %s",
-			             path, strerror (errno));
+			p11_message_err (errno, "couldn't remove file: %s", path);
 			ret = false;
 			break;
 		}
@@ -522,8 +510,7 @@ p11_save_finish_directory (p11_save_dir *dir,
 		/* Try to set the mode of the directory to readable */
 		if (ret && chmod (dir->path, S_IRUSR | S_IXUSR | S_IRGRP |
 		                             S_IXGRP | S_IROTH | S_IXOTH) < 0) {
-			p11_message ("couldn't set directory permissions: %s: %s",
-			             dir->path, strerror (errno));
+			p11_message_err (errno, "couldn't set directory permissions: %s", dir->path);
 			ret = false;
 		}
 #endif /* OS_UNIX */
diff --git a/tools/tests/test.c b/tools/tests/test.c
index 589e7e9..2aaf2c7 100644
--- a/tools/tests/test.c
+++ b/tools/tests/test.c
@@ -257,8 +257,7 @@ test_temp_directory (const char *templ)
 		return NULL;
 
 	if (!mkdtemp (directory)) {
-		p11_message ("couldn't create temp directory: %s: %s",
-		             directory, strerror (errno));
+		p11_message_err (errno, "couldn't create temp directory: %s", directory);
 		free (directory);
 		assert (0 && "not reached");
 	}
diff --git a/trust/parser.c b/trust/parser.c
index eb0b096..d1fa4e1 100644
--- a/trust/parser.c
+++ b/trust/parser.c
@@ -781,7 +781,7 @@ p11_parse_file (p11_parser *parser,
 
 	map = p11_mmap_open (filename, &data, &size);
 	if (map == NULL) {
-		p11_message ("couldn't open and map file: %s: %s", filename, strerror (errno));
+		p11_message_err (errno, "couldn't open and map file: %s", filename);
 		return P11_PARSE_FAILURE;
 	}
 
diff --git a/trust/token.c b/trust/token.c
index f48f66b..d5fab8b 100644
--- a/trust/token.c
+++ b/trust/token.c
@@ -105,8 +105,7 @@ loader_load_directory (p11_token *token,
 	/* First we load all the modules */
 	dir = opendir (directory);
 	if (!dir) {
-		p11_message ("couldn't list directory: %s: %s",
-		             directory, strerror (errno));
+		p11_message_err (errno, "couldn't list directory: %s", directory);
 		return 0;
 	}
 
@@ -116,7 +115,7 @@ loader_load_directory (p11_token *token,
 		return_val_if_fail (path != NULL, -1);
 
 		if (stat (path, &sb) < 0) {
-			p11_message ("couldn't stat path: %s", path);
+			p11_message_err (errno, "couldn't stat path: %s", path);
 
 		} else if (!S_ISDIR (sb.st_mode)) {
 			ret = loader_load_file (token, path, &sb, flags);
@@ -164,8 +163,7 @@ loader_load_path (p11_token *token,
 			p11_message ("trust certificate path does not exist: %s",
 			             path);
 		} else {
-			p11_message ("cannot access trust certificate path: %s: %s",
-			             path, strerror (errno));
+			p11_message_err (errno, "cannot access trust certificate path: %s", path);
 		}
 
 		return 0;