Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-26 Thread walter harms


Am 20.04.2012 23:18, schrieb Chase Douglas:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.
 
 Signed-off-by: Chase Douglas chase.doug...@canonical.com
 ---
  AuFileName.c |   17 -
  1 files changed, 16 insertions(+), 1 deletions(-)
 
 diff --git a/AuFileName.c b/AuFileName.c
 index f384f75..bc7b177 100644
 --- a/AuFileName.c
 +++ b/AuFileName.c
 @@ -31,13 +31,22 @@ in this Software without prior written authorization from 
 The Open Group.
  #include X11/Xos.h
  #include stdlib.h
  
 +static char *buf = NULL;
 +
 +static void
 +free_filename_buffer(void)
 +{
 +free(buf);
 +buf = NULL;
 +}
 +
  char *
  XauFileName (void)
  {
  const char *slashDotXauthority = /.Xauthority;
  char*name;
 -static char  *buf;
  static int   bsize;
 +static int atexit_registered = 0;
  #ifdef WIN32
  chardir[128];
  #endif
 @@ -64,6 +73,12 @@ XauFileName (void)
   buf = malloc ((unsigned) size);
   if (!buf)
   return NULL;
 +
 +if (!atexit_registered) {
 +atexit(free_filename_buffer);
 +atexit_registered = 1;
 +}
 +
   bsize = size;
  }
  strcpy (buf, name);


Is XauFileName() a case for Xasprintf() ?

re,
 wh

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-26 Thread Alan Coopersmith
On 04/26/12 01:30 AM, walter harms wrote:
 Is XauFileName() a case for Xasprintf() ?

No, because Xasprintf() only exists in the X server, while XauFilename
is also used in client side code (libX11, xauth, etc.).

-- 
-Alan Coopersmith-  alan.coopersm...@oracle.com
 Oracle Solaris Engineering - http://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-21 Thread Mark Kettenis
 From: Chase Douglas chase.doug...@canonical.com
 Date: Fri, 20 Apr 2012 14:18:59 -0700
 
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

Which is fairly pointless given that the program is exiting anyway.

I guess you're using some sort of tool to detect memory leaks.  I
don't think the additional code bloat is worth it to avoid what's
arguably a false positive.

 Signed-off-by: Chase Douglas chase.doug...@canonical.com
 ---
  AuFileName.c |   17 -
  1 files changed, 16 insertions(+), 1 deletions(-)
 
 diff --git a/AuFileName.c b/AuFileName.c
 index f384f75..bc7b177 100644
 --- a/AuFileName.c
 +++ b/AuFileName.c
 @@ -31,13 +31,22 @@ in this Software without prior written authorization from 
 The Open Group.
  #include X11/Xos.h
  #include stdlib.h
  
 +static char *buf = NULL;
 +
 +static void
 +free_filename_buffer(void)
 +{
 +free(buf);
 +buf = NULL;
 +}
 +
  char *
  XauFileName (void)
  {
  const char *slashDotXauthority = /.Xauthority;
  char*name;
 -static char  *buf;
  static int   bsize;
 +static int atexit_registered = 0;
  #ifdef WIN32
  chardir[128];
  #endif
 @@ -64,6 +73,12 @@ XauFileName (void)
   buf = malloc ((unsigned) size);
   if (!buf)
   return NULL;
 +
 +if (!atexit_registered) {
 +atexit(free_filename_buffer);
 +atexit_registered = 1;
 +}
 +
   bsize = size;
  }
  strcpy (buf, name);
 -- 
 1.7.9.1
 
 ___
 xorg-devel@lists.x.org: X.Org development
 Archives: http://lists.x.org/archives/xorg-devel
 Info: http://lists.x.org/mailman/listinfo/xorg-devel
 
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


[PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Chase Douglas
XauFileName() may allocate and return a static buffer. The only
way to ensure it is freed is to deallocate it when the program exits.

Signed-off-by: Chase Douglas chase.doug...@canonical.com
---
 AuFileName.c |   17 -
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/AuFileName.c b/AuFileName.c
index f384f75..bc7b177 100644
--- a/AuFileName.c
+++ b/AuFileName.c
@@ -31,13 +31,22 @@ in this Software without prior written authorization from 
The Open Group.
 #include X11/Xos.h
 #include stdlib.h
 
+static char *buf = NULL;
+
+static void
+free_filename_buffer(void)
+{
+free(buf);
+buf = NULL;
+}
+
 char *
 XauFileName (void)
 {
 const char *slashDotXauthority = /.Xauthority;
 char*name;
-static char*buf;
 static int bsize;
+static int atexit_registered = 0;
 #ifdef WIN32
 chardir[128];
 #endif
@@ -64,6 +73,12 @@ XauFileName (void)
buf = malloc ((unsigned) size);
if (!buf)
return NULL;
+
+if (!atexit_registered) {
+atexit(free_filename_buffer);
+atexit_registered = 1;
+}
+
bsize = size;
 }
 strcpy (buf, name);
-- 
1.7.9.1

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Matt Turner
On Fri, Apr 20, 2012 at 5:18 PM, Chase Douglas
chase.doug...@canonical.com wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

 Signed-off-by: Chase Douglas chase.doug...@canonical.com
 ---
  AuFileName.c |   17 -
  1 files changed, 16 insertions(+), 1 deletions(-)

 diff --git a/AuFileName.c b/AuFileName.c
 index f384f75..bc7b177 100644
 --- a/AuFileName.c
 +++ b/AuFileName.c
 @@ -31,13 +31,22 @@ in this Software without prior written authorization from 
 The Open Group.
  #include X11/Xos.h
  #include stdlib.h

 +static char *buf = NULL;
 +
 +static void
 +free_filename_buffer(void)
 +{
 +    free(buf);
 +    buf = NULL;
 +}
 +

Strictly speaking, you don't need to set buf = NULL in either place.
static variables are automatically initialized, and I can't imagine
why the program should care what the value of buf is when it's
exiting. :)

Doesn't really matter though.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Alan Coopersmith
On 04/20/12 02:18 PM, Chase Douglas wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

...at which point it's automatically freed.   Is this just trying to
silence some sort of memory leak checking?

-- 
-Alan Coopersmith-  alan.coopersm...@oracle.com
 Oracle Solaris Engineering - http://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Chase Douglas
On 04/20/2012 02:35 PM, Alan Coopersmith wrote:
 On 04/20/12 02:18 PM, Chase Douglas wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.
 
 ...at which point it's automatically freed.   Is this just trying to
 silence some sort of memory leak checking?

Yes. I'm running valgrind and trying to solve leaks. I realize this is
not a real issue, but it's essentially like a compiler warning.
Reducing warnings and errors helps highlight where there are real problems.

-- Chase
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Chase Douglas
On 04/20/2012 02:31 PM, Matt Turner wrote:
 On Fri, Apr 20, 2012 at 5:18 PM, Chase Douglas
 chase.doug...@canonical.com wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

 Signed-off-by: Chase Douglas chase.doug...@canonical.com
 ---
  AuFileName.c |   17 -
  1 files changed, 16 insertions(+), 1 deletions(-)

 diff --git a/AuFileName.c b/AuFileName.c
 index f384f75..bc7b177 100644
 --- a/AuFileName.c
 +++ b/AuFileName.c
 @@ -31,13 +31,22 @@ in this Software without prior written authorization 
 from The Open Group.
  #include X11/Xos.h
  #include stdlib.h

 +static char *buf = NULL;
 +
 +static void
 +free_filename_buffer(void)
 +{
 +free(buf);
 +buf = NULL;
 +}
 +
 
 Strictly speaking, you don't need to set buf = NULL in either place.
 static variables are automatically initialized, and I can't imagine
 why the program should care what the value of buf is when it's
 exiting. :)

I like being pedantic when it comes to values that could crash your
program if you make an unrelated change, like removing the static keyword.

-- Chase
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Alan Coopersmith
On 04/20/12 02:56 PM, Chase Douglas wrote:
 On 04/20/2012 02:35 PM, Alan Coopersmith wrote:
 On 04/20/12 02:18 PM, Chase Douglas wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

 ...at which point it's automatically freed.   Is this just trying to
 silence some sort of memory leak checking?
 
 Yes. I'm running valgrind and trying to solve leaks. I realize this is
 not a real issue, but it's essentially like a compiler warning.
 Reducing warnings and errors helps highlight where there are real problems.

Right, just trying to make sure that I understood what problem you were
trying to solve (quieting false positives so real problems are easier to
find), and the patch seems to be a reasonable way to do that, so:

Reviewed-by: Alan Coopersmith alan.coopersm...@oracle.com

-- 
-Alan Coopersmith-  alan.coopersm...@oracle.com
 Oracle Solaris Engineering - http://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Chase Douglas
On 04/20/2012 03:06 PM, Alan Coopersmith wrote:
 On 04/20/12 02:56 PM, Chase Douglas wrote:
 On 04/20/2012 02:35 PM, Alan Coopersmith wrote:
 On 04/20/12 02:18 PM, Chase Douglas wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

 ...at which point it's automatically freed.   Is this just trying to
 silence some sort of memory leak checking?

 Yes. I'm running valgrind and trying to solve leaks. I realize this is
 not a real issue, but it's essentially like a compiler warning.
 Reducing warnings and errors helps highlight where there are real problems.
 
 Right, just trying to make sure that I understood what problem you were
 trying to solve (quieting false positives so real problems are easier to
 find), and the patch seems to be a reasonable way to do that, so:
 
 Reviewed-by: Alan Coopersmith alan.coopersm...@oracle.com

So libXau is listed as unmaintained:

http://cgit.freedesktop.org/xorg/doc/xorg-docs/tree/MAINTAINERS#n176

but I see you have been applying patches. Will you be picking this up? I
can apply it, too. Either way.

-- Chase

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Alan Coopersmith
On 04/20/12 03:11 PM, Chase Douglas wrote:
 On 04/20/2012 03:06 PM, Alan Coopersmith wrote:
 On 04/20/12 02:56 PM, Chase Douglas wrote:
 On 04/20/2012 02:35 PM, Alan Coopersmith wrote:
 On 04/20/12 02:18 PM, Chase Douglas wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

 ...at which point it's automatically freed.   Is this just trying to
 silence some sort of memory leak checking?

 Yes. I'm running valgrind and trying to solve leaks. I realize this is
 not a real issue, but it's essentially like a compiler warning.
 Reducing warnings and errors helps highlight where there are real problems.

 Right, just trying to make sure that I understood what problem you were
 trying to solve (quieting false positives so real problems are easier to
 find), and the patch seems to be a reasonable way to do that, so:

 Reviewed-by: Alan Coopersmith alan.coopersm...@oracle.com
 
 So libXau is listed as unmaintained:
 
 http://cgit.freedesktop.org/xorg/doc/xorg-docs/tree/MAINTAINERS#n176
 
 but I see you have been applying patches. Will you be picking this up? I
 can apply it, too. Either way.

You can go ahead and push directly.   I tend to worry more about picking up
patches from people without direct git write access.

-- 
-Alan Coopersmith-  alan.coopersm...@oracle.com
 Oracle Solaris Engineering - http://blogs.oracle.com/alanc
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel


Re: [PATCH libXau] Free XauFileName() static buffer at exit

2012-04-20 Thread Chase Douglas
On 04/20/2012 03:15 PM, Alan Coopersmith wrote:
 On 04/20/12 03:11 PM, Chase Douglas wrote:
 On 04/20/2012 03:06 PM, Alan Coopersmith wrote:
 On 04/20/12 02:56 PM, Chase Douglas wrote:
 On 04/20/2012 02:35 PM, Alan Coopersmith wrote:
 On 04/20/12 02:18 PM, Chase Douglas wrote:
 XauFileName() may allocate and return a static buffer. The only
 way to ensure it is freed is to deallocate it when the program exits.

 ...at which point it's automatically freed.   Is this just trying to
 silence some sort of memory leak checking?

 Yes. I'm running valgrind and trying to solve leaks. I realize this is
 not a real issue, but it's essentially like a compiler warning.
 Reducing warnings and errors helps highlight where there are real problems.

 Right, just trying to make sure that I understood what problem you were
 trying to solve (quieting false positives so real problems are easier to
 find), and the patch seems to be a reasonable way to do that, so:

 Reviewed-by: Alan Coopersmith alan.coopersm...@oracle.com

 So libXau is listed as unmaintained:

 http://cgit.freedesktop.org/xorg/doc/xorg-docs/tree/MAINTAINERS#n176

 but I see you have been applying patches. Will you be picking this up? I
 can apply it, too. Either way.
 
 You can go ahead and push directly.   I tend to worry more about picking up
 patches from people without direct git write access.

Done.

Thanks!

-- Chase
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel