XOpenDisplay() lags memory? (valgrind)

Barton C Massey bart at cs.pdx.edu
Thu Feb 22 23:24:50 PST 2007


Attached is a patch that should solve your problem, but that
preserves the old, leaky behavior of libXau for callers of
XauFileName() outside of libXau itself.  It's fairly ugly.

Please file a bug report, attach this patch to it, and then
let us know if it fixes the problem.

    Bart Massey
    Assoc. Prof. Computer Science
    Portland State University
    bart at cs.pdx.edu

In message <200702222100.45562.trapni at gentoo.org> you wrote:
> --===============0205124584==
> Content-Type: multipart/signed; boundary="nextPart2707426.aIc8AGJDYs";
> 	protocol="application/pgp-signature"; micalg=pgp-sha1
> Content-Transfer-Encoding: 7bit
> 
> --nextPart2707426.aIc8AGJDYs
> Content-Type: text/plain;
>   charset="us-ascii"
> Content-Transfer-Encoding: quoted-printable
> Content-Disposition: inline
> 
> Hi all,
> 
> I'm cleaning up a little, so I valgrind'ed, and this is one that's left:
> 
> =3D=3D24707=3D=3D 25 bytes in 1 blocks are still reachable in loss record 2=
>  of 3
> =3D=3D24707=3D=3D    at 0x4A20CFB: malloc=20
> (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
> =3D=3D24707=3D=3D    by 0x510BFD6: XauFileName (in /usr/lib64/libXau.so.6.0=
> =2E0)
> =3D=3D24707=3D=3D    by 0x510C212: XauGetBestAuthByAddr=20
> (in /usr/lib64/libXau.so.6.0.0)
> =3D=3D24707=3D=3D    by 0x4F50598: _X11TransConnectDisplay=20
> (in /usr/lib64/libX11.so.6.2.0)
> =3D=3D24707=3D=3D    by 0x4F37455: XOpenDisplay (in /usr/lib64/libX11.so.6.=
> 2.0)
> =3D=3D24707=3D=3D    by 0x401539: main (encode-stream.cpp:88)
> =3D=3D24707=3D=3D
> 
> However, I definitely did call XCloseDisplay(dpy) at the end, that's why I=
> =20
> feel so confusing.
> 
> Is this a bug, or could I have been missing something?
> 
> Thanks in advance,
> Christian Parpart.
> 
> p.s.: Xorg 7.2
> 
> --nextPart2707426.aIc8AGJDYs
> Content-Type: application/pgp-signature
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.2 (GNU/Linux)
> 
> iD8DBQBF3fZtPpa2GmDVhK0RAqA/AJ4hlSjg0ZtNMBJebYzWbZ7197XG4QCfe5X2
> xGwR2vJ0bW6eaMdzbatT/S0=
> =/YTR
> -----END PGP SIGNATURE-----
> 
> --nextPart2707426.aIc8AGJDYs--
> 
> --===============0205124584==
> Content-Type: text/plain; charset="us-ascii"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline
> 
> _______________________________________________
> xorg mailing list
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg
> --===============0205124584==--

-------------- next part --------------
>From b15d7861c35e16555430838cc78c8cd72630236f Mon Sep 17 00:00:00 2001
From: Bart Massey <bart at cs.pdx.edu>
Date: Thu, 22 Feb 2007 23:01:02 -0800
Subject: [PATCH] cleaned up XauFileName() memory handling; split out XauOpenAuth()


Signed-off-by: Bart Massey <bart at cs.pdx.edu>
---
 AuFileName.c        |   15 ++++++++++++---
 AuGetAddr.c         |   11 ++---------
 AuGetBest.c         |    9 +--------
 Makefile.am         |    5 ++++-
 Xau.man             |   19 ++++++++++++++++---
 include/X11/Xauth.h |    4 ++++
 6 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/AuFileName.c b/AuFileName.c
index b21b048..fb9b122 100644
--- a/AuFileName.c
+++ b/AuFileName.c
@@ -34,12 +34,13 @@ in this Software without prior written authorization from The Open Group.
 #include <X11/Xos.h>
 #include <stdlib.h>
 
+static char	*buf;
+
 char *
 XauFileName (void)
 {
     const char *slashDotXauthority = "/.Xauthority";
     char    *name;
-    static char	*buf;
     static int	bsize;
 #ifdef WIN32
     char    dir[128];
@@ -62,8 +63,7 @@ XauFileName (void)
     }
     size = strlen (name) + strlen(&slashDotXauthority[1]) + 2;
     if (size > bsize) {
-	if (buf)
-	    free (buf);
+        XauFreeFileName();
 	buf = malloc ((unsigned) size);
 	if (!buf)
 	    return NULL;
@@ -73,3 +73,12 @@ XauFileName (void)
     strcat (buf, slashDotXauthority + (name[1] == '\0' ? 1 : 0));
     return buf;
 }
+
+void
+XauFreeFileName (void)
+{
+  if (buf) {
+    free (buf);
+    buf = 0;
+  }
+}
diff --git a/AuGetAddr.c b/AuGetAddr.c
index 6275531..32c8f34 100644
--- a/AuGetAddr.c
+++ b/AuGetAddr.c
@@ -65,18 +65,11 @@ unsigned short	name_length,
 #endif
 _Xconst char*	name)
 {
-    FILE    *auth_file;
-    char    *auth_name;
+    FILE    *auth_file = XauOpenAuth();
     Xauth   *entry;
 
-    auth_name = XauFileName ();
-    if (!auth_name)
-	return NULL;
-    if (access (auth_name, R_OK) != 0)		/* checks REAL id */
-	return NULL;
-    auth_file = fopen (auth_name, "rb");
     if (!auth_file)
-	return NULL;
+        return NULL;
     for (;;) {
 	entry = XauReadAuth (auth_file);
 	if (!entry)
diff --git a/AuGetBest.c b/AuGetBest.c
index ae2b748..68d9aa1 100644
--- a/AuGetBest.c
+++ b/AuGetBest.c
@@ -70,8 +70,7 @@ XauGetBestAuthByAddr (
     char**		types,
     _Xconst int*	type_lengths)
 {
-    FILE    *auth_file;
-    char    *auth_name;
+    FILE    *auth_file = XauOpenAuth();
     Xauth   *entry;
     Xauth   *best;
     int	    best_type;
@@ -81,12 +80,6 @@ XauGetBestAuthByAddr (
     unsigned short	fully_qual_address_length;
 #endif
 
-    auth_name = XauFileName ();
-    if (!auth_name)
-	return NULL;
-    if (access (auth_name, R_OK) != 0)		/* checks REAL id */
-	return NULL;
-    auth_file = fopen (auth_name, "rb");
     if (!auth_file)
 	return NULL;
 
diff --git a/Makefile.am b/Makefile.am
index b597622..42f4c31 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,6 +14,7 @@ libXau_la_SOURCES = \
 	AuGetAddr.c \
 	AuGetBest.c \
 	AuLock.c \
+	AuOpenAuth.c \
 	AuRead.c \
 	AuUnlock.c \
 	AuWrite.c
@@ -65,13 +66,15 @@ EXTRA_DIST = xau.pc.in $(libman_PRE) ChangeLog
 
 Xau_shadows =                               \
     XauFileName                             \
+    XauFreeFileName                         \
     XauReadAuth                             \
     XauLockAuth                             \
     XauUnlockAuth                           \
     XauWriteAuth                            \
     XauDisposeAuth                          \
     XauGetAuthByAddr                        \
-    XauGetBestAuthByAddr
+    XauGetBestAuthByAddr                    \
+    XauOpenAuth
 
 shadows.DONE:
 	-rm -f $(Xau_shadows:=. at LIB_MAN_SUFFIX@)
diff --git a/Xau.man b/Xau.man
index a336523..928c7fa 100644
--- a/Xau.man
+++ b/Xau.man
@@ -30,7 +30,8 @@
 .\"
 .TH Xau 3 __xorgversion__
 .SH NAME
-Xau library: XauFileName, XauReadAuth, XauLockAuth, XauUnlockAuth,
+Xau library: XauFileName, XauFreeFileName, XauOpenAuth,
+XauReadAuth, XauLockAuth, XauUnlockAuth,
 XauWriteAuth, XauDisposeAuth,
 XauGetAuthByAddr, XauGetBestAuthByAddr \- X authority database routines
 .SH SYNOPSIS
@@ -53,6 +54,10 @@ typedef struct xauth {
 .HP
 char *XauFileName (void); 
 .HP
+void XauFreeFileName (void); 
+.HP
+FILE *XauOpenAuth (void);
+.HP
 Xauth *XauReadAuth (FILE *\fIauth_file\fP\^); 
 .HP
 int XauWriteAuth (FILE *\fIauth_file\fP, Xauth *\fIauth\fP\^); 
@@ -77,8 +82,16 @@ int XauDisposeAuth (Xauth *\fIauth\fP\^);
 .PP
 \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.
+$HOME/.Xauthority.    The routine returns NULL on error.
+The returned name is a static pointer to heap-allocated memory, and
+should be freed by a call to \fBXauFreeFileName\fP when no
+longer needed.
+.PP
+\fBXauFreeFileName\fP frees the filename allocated by the
+most recent call to \fBXauFileName\fP, if any.
+.PP
+\fBXauOpenAuth\fP opens the authorization file and returns a
+pointer to it, or NULL on error.
 .PP
 \fBXauReadAuth\fP reads the next entry from \fIauth_file\fP.  The entry is
 \fBnot\fP statically allocated and should be freed by calling
diff --git a/include/X11/Xauth.h b/include/X11/Xauth.h
index 181618a..d2552fc 100644
--- a/include/X11/Xauth.h
+++ b/include/X11/Xauth.h
@@ -61,6 +61,10 @@ _XFUNCPROTOBEGIN
 
 char *XauFileName(void);
 
+void XauFreeFileName(void);
+
+FILE* XauOpenAuth(void);
+
 Xauth *XauReadAuth(
 FILE*	/* auth_file */
 );
-- 
1.4.4.2



More information about the xorg mailing list