Re: [PATCH libXau] Free XauFileName() static buffer at exit
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
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
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
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
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
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
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
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
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
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
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
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