[PATCH v2 1/2] [libXau] XauGetFileName: added a thread-safe variant of XauFileName

Erkki Seppälä erkki.seppala at vincit.fi
Wed Mar 30 01:53:53 PDT 2011


XauGetFileName has argument char **buffer, which can be used to
provide an existing buffer for XauGetFileName to store the result
in. *buffer can be NULL, in which case a newly allocated block of
memory will be allocated and stored at *buffer. The function also
provides means to signal the required buffer length back should the
buffer be too small. All different situations are signaled with a
return value enum XauthGetFnStatus, where XAUTH_GETFN_OK = 0.

XauFileName is reimplemented in terms of XauGetFileName. As a side
effect it might reduce the chance of heap corruption. However, this
still doesn't mean XauFileName is anywhere near thread safe. LibXau
has been internally fixed to use XauGetFileName, so you should be safe
if you don't call XauFileName directly.

NetBSD getenv_r is also supported. Unfortunately this support has only
been tested with a simulation function under Linux, as I don't have
access to a NetBSD host, and neither FreeBSD nor Solaris provide this
function. Linux getenv is already thread safe (assuming environment is
not changed), as it simply returns a pointer inside the environment in
process address space. Also Solaris getenv is thread safe according to
its documentation.

Signed-off-by: Erkki Seppälä <erkki.seppala at vincit.fi>
Reviewed-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
---
 AuFileName.c        |  163 ++++++++++++++++++++++++++++++++++++++++++---------
 configure.ac        |    5 ++
 include/X11/Xauth.h |   38 ++++++++++++
 man/Xau.man         |   57 +++++++++++++++++-
 4 files changed, 234 insertions(+), 29 deletions(-)

diff --git a/AuFileName.c b/AuFileName.c
index f384f75..753d0d4 100644
--- a/AuFileName.c
+++ b/AuFileName.c
@@ -30,43 +30,152 @@ in this Software without prior written authorization from The Open Group.
 #include <X11/Xauth.h>
 #include <X11/Xos.h>
 #include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+
+#ifdef USE_MTSAFE_GETENV
+/**
+ * @brief xau_getenv returns the contents of an environment
+ * variable. 
+ * 
+ * Previously returned value may be trashed by this call!
+ *
+ * @returns If the value is found, the returned string may be
+ * allocated from heap and must be released with the function
+ * xau_freeenv. If the value is not found, NULL will be returned.
+ */
+static const char *
+xau_getenv(const char *name)
+{
+    size_t  len	= 64;
+    char   *buffer = malloc (len);
+    int	    rc = 0;
+
+    while (buffer
+	   && (rc = getenv_r (name, buffer, len)) == -1
+	   && errno == ERANGE) {
+	len *= 2;
+	free (buffer);
+	buffer = malloc (len);
+    }
+    return rc == 0 ? buffer : NULL;
+}
+
+/**
+ * @brief xau_freeenv releases a variable stored in heap by
+ * xau_getenv. This function may be a non-op if the environment
+ * variables do not need to be copied.
+ */
+static void
+xau_freeenv(char *value)
+{
+    free (value);
+}
+#else /* #if USE_MTSAFE_GETENV */
+static const char *
+xau_getenv(const char *name)
+{
+    return getenv ((char*) name);
+}
+
+static void
+xau_freeenv(const char *value)
+{
+    (void) value;
+}
+#endif
 
 char *
 XauFileName (void)
 {
-    const char *slashDotXauthority = "/.Xauthority";
-    char    *name;
     static char	*buf;
-    static int	bsize;
+    static int size;
+    int tries;
+
+    for (tries = 2; tries > 0; --tries) {
+	switch (XauGetFileName (&buf, &size)) {
+	case XAUTH_GETFN_OK:
+	    return buf;
+	case XAUTH_GETFN_NOT_FOUND:
+	case XAUTH_GETFN_OUT_OF_MEMORY:
+	    return NULL;
+	case XAUTH_GETFN_BUFFER_TOO_SMALL:
+	    buf = realloc (buf, size);
+	}
+    }
+
+    return NULL;
+}
+
+XauthGetFnStatus
+XauGetFileName (char **buf, int *buf_len)
+{
+    static const char *slashDotXauthority = "/.Xauthority";
+    int		       size;
+    /* only one of xauthority and home is non-NULL at any point of the
+       function to handle the fact that xau_getenv may trash the
+       contents of its previous return value. */
+    char	      *xauthority	  = NULL;
+    char	      *home		  = NULL;
+    const char	      *relativeXauthority = NULL;
+    int		       rc;
 #ifdef WIN32
-    char    dir[128];
+    char	       dir[128];
 #endif
-    int	    size;
 
-    if ((name = getenv ("XAUTHORITY")))
-	return name;
-    name = getenv ("HOME");
-    if (!name) {
+    /* if XAUTHORITY is provided, use that. (Still do buffer
+       handling.) */
+    xauthority = xau_getenv ("XAUTHORITY");
+    if (xauthority) {
+	size = strlen (xauthority) + 1;
+    } else {
+	home = xau_getenv ("HOME");
+	if (!home) {
 #ifdef WIN32
-	(void) strcpy (dir, "/users/");
-	if ((name = getenv("USERNAME"))) {
-	    (void) strcat (dir, name);
-	    name = dir;
-	}
-	if (!name)
+	    char *username = xau_getenv ("USERNAME");
+	    if (username) {
+		snprintf (dir, sizeof(dir), "/users/%s", username);
+		xau_freeenv (username);
+		home = dir;
+	    } else
 #endif
-	return NULL;
+	    {
+		rc = XAUTH_GETFN_NOT_FOUND;
+		goto cleanup;
+	    }
+	}
+	relativeXauthority = slashDotXauthority + ((home[0] && !home[1]) ? 1 : 0);
+	size = strlen (home) + strlen (relativeXauthority) + 1;
     }
-    size = strlen (name) + strlen(&slashDotXauthority[1]) + 2;
-    if (size > bsize) {
-	if (buf)
-	    free (buf);
-	buf = malloc ((unsigned) size);
-	if (!buf)
-	    return NULL;
-	bsize = size;
+
+    /* ensure *buf is large enough for the result */
+    if (!buf_len) {
+	char *ptr = malloc ((unsigned) size);
+	if (!ptr) {
+	    rc = XAUTH_GETFN_OUT_OF_MEMORY;
+	    goto cleanup;
+	}
+	*buf = ptr;
+    } else if (*buf_len < size) {
+	*buf_len = size;
+	rc = XAUTH_GETFN_BUFFER_TOO_SMALL;
+	goto cleanup;
     }
-    strcpy (buf, name);
-    strcat (buf, slashDotXauthority + (name[1] == '\0' ? 1 : 0));
-    return buf;
+    if (buf_len)
+	*buf_len = size;
+
+    if (xauthority) {
+	strcpy (*buf, xauthority);
+    } else {
+	strcpy (*buf, home);
+	strcat (*buf, relativeXauthority);
+    }
+    rc = XAUTH_GETFN_OK;
+ cleanup:
+    xau_freeenv (xauthority);
+#ifdef WIN32
+    if (home != dir)
+#endif
+	xau_freeenv (home);
+    return rc;
 }
diff --git a/configure.ac b/configure.ac
index 09a872e..e1cece1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -71,6 +71,11 @@ if test "x$xthreads" = "xyes" ; then
     XAU_CFLAGS="$XAU_CFLAGS $XTHREAD_CFLAGS"
 fi
 
+AC_CHECK_LIB(c, getenv_r, [mtsafe_getenv=yes])
+if test "x$mtsafe_getenv"; then
+    AC_DEFINE(XUSE_MTSAFE_GETENV, 1, [Use getenv_r in place of getenv])
+fi
+
 # Allow checking code with lint, sparse, etc.
 XORG_WITH_LINT
 XORG_LINT_LIBRARY([Xau])
diff --git a/include/X11/Xauth.h b/include/X11/Xauth.h
index f57a1b3..47876e2 100644
--- a/include/X11/Xauth.h
+++ b/include/X11/Xauth.h
@@ -57,6 +57,44 @@ _XFUNCPROTOBEGIN
 
 char *XauFileName(void);
 
+typedef enum {
+    XAUTH_GETFN_OK,		  /* OK, .Xauthority found */
+    XAUTH_GETFN_NOT_FOUND,	  /* .Xauthority not found */
+    XAUTH_GETFN_BUFFER_TOO_SMALL, /* Provided buffer was too small */
+    XAUTH_GETFN_OUT_OF_MEMORY	  /* Out of memory */
+} XauthGetFnStatus;
+
+/**
+ * @brief A thread-safe variant of XauFileName.
+ *
+ * @param buffer The buffer-parameter must point to a character
+ * pointer, which may be NULL. If *buffer_size is NULL, memory will be
+ * dynamically allocated for the return value and its address will be
+ * placed at *buffer regardless of its contents.
+ *
+ * @param buffer_size If buffer_size is not NULL, *buffer_size will be
+ * used to determine the size of the provided buffer. After the call,
+ * *buffer_size will contain the number of bytes required to store the
+ * result. If the function returns XAUTH_GETFN_BUFFER_TOO_SMALL, you
+ * may use this value to determine the number of bytes you need to
+ * allocate for the buffer. On success *buffer_size contains the length
+ * of the returned string with the terminating nil-character included.
+ *
+ * @return The function returns XAUTH_GETFN_OK when it succeeds in
+ * finding the location of the .Xauthority, and the location has been
+ * stored at *buffer (and its size, if buffer_size is not NULL, at
+ * *buffer_size). If the function returns XAUTH_GETFN_NOT_FOUND, it
+ * could not determine where .Xauthority locates.
+ * XAUTH_GETFN_BUFFER_TOO_SMALL indicates that the provided
+ * *buffer_size was too small to fit the result in; *buffer_size now
+ * contains the number of bytes required. XAUTH_GETFN_OUT_OF_MEMORY
+ * indicates that an error occurred while allocating memory.
+ */
+XauthGetFnStatus XauGetFileName(
+char ** /* buffer */,
+int *   /* buffer_size */
+);
+
 Xauth *XauReadAuth(
 FILE*	/* auth_file */
 );
diff --git a/man/Xau.man b/man/Xau.man
index 46d4a19..e8b1f9e 100644
--- a/man/Xau.man
+++ b/man/Xau.man
@@ -25,7 +25,7 @@
 .\"
 .TH Xau __libmansuffix__ __xorgversion__
 .SH NAME
-Xau library: XauFileName, XauReadAuth, XauLockAuth, XauUnlockAuth,
+Xau library: XauFileName, XauGetFileName, XauReadAuth, XauLockAuth, XauUnlockAuth,
 XauWriteAuth, XauDisposeAuth,
 XauGetAuthByAddr, XauGetBestAuthByAddr \- X authority database routines
 .SH SYNOPSIS
@@ -45,9 +45,17 @@ typedef struct xauth {
 	char	*data;
 } Xauth;
 
+typedef enum {
+	XAUTH_GETFN_OK,
+	XAUTH_GETFN_NOT_FOUND,
+	XAUTH_GETFN_BUFFER_TOO_SMALL,
+	XAUTH_GETFN_OUT_OF_MEMORY
+} XauthGetFnStatus;
 .HP
 char *XauFileName (void);
 .HP
+XauthGetFnStatus XauGetFileName (char **buffer, int *buffer_size);
+.HP
 Xauth *XauReadAuth (FILE *\fIauth_file\fP\^);
 .HP
 int XauWriteAuth (FILE *\fIauth_file\fP, Xauth *\fIauth\fP\^);
@@ -74,7 +82,52 @@ int XauDisposeAuth (Xauth *\fIauth\fP\^);
 \fBXauFileName\fP generates the default authorization file name by first
 checking the XAUTHORITY environment variable if set, else it returns
 $HOME/.Xauthority.  This name is statically allocated and should
-not be freed.
+not be freed. Note that this function is not thread safe and XauGetFileName
+should be used in applications and libraries that wish to be thread safe.
+.PP
+\fBXauGetFileName\fP is similar to XauFileName, except that this
+version is thread safe. The name is stored into the buffer provided,
+or if no such buffer is provided a new one will be dynamically
+allocated (which should be freed).
+.PP
+\fBXauGetFileName\fP accepts the following parameters:
+.nf
+.ta .5i 2i
+
+	buffer	The buffer-parameter must point to a character pointer,
+		which may be NULL. If *buffer_size is NULL, memory
+		will be dynamically allocated for the return value and
+		its address will be placed at *buffer regardless of
+		the contents of *buffer.
+
+	buffer_size	If buffer_size is not NULL, *buffer_size will be
+		used to determine the size of the buffer
+		provided. After the call, *buffer_size will contain
+		the number of bytes required to store the result. If
+		the function returns XAUTH_GETFN_BUFFER_TOO_SMALL, you
+		may use this value to determine the number of bytes
+		you need to allocate for the buffer. On success
+		*buffer_size contains the length of the returned
+		string with the terminating nil-character included.
+.fi
+.PP
+\fBXauGetFileName\fP returns one of the following return values:
+.nf
+.ta .5i 2i
+
+	XAUTH_GETFN_OK	The location of .Xauthority was found and
+		it has been stored at *buffer. Its length has been
+		stored at *buffer_size, if buffer_size is not NULL.
+
+	XAUTH_GETFN_NOT_FOUND	 .Xauthority could not be found.
+
+	XAUTH_GETFN_BUFFER_TOO_SMALL The provided buffer was too
+		small.  You may call XauGetFileName again with a
+		larger buffer.
+
+	XAUTH_GETFN_OUT_OF_MEMORY There was an error while allocating
+		memory.
+.fi
 .PP
 \fBXauReadAuth\fP reads the next entry from \fIauth_file\fP.  The entry is
 \fBnot\fP statically allocated and should be freed by calling
-- 
1.7.0.4



More information about the xorg-devel mailing list