bug#26628: [PATCH] Fix memory leak of cwd string in emacsclient

bug#26628: [PATCH] Fix memory leak of cwd string in emacsclient

[Anders Waldenborg]

[-- Attachment #1.1: Type: text/plain, Size: 60 bytes --]

The attached patch fixes a tiny memory leak in emacsclient.

[-- Attachment #1.2: Type: text/html, Size: 111 bytes --]

[-- Attachment #2: 0001-Fix-memory-leak-of-cwd-string-in-emacsclient.patch --]
[-- Type: text/x-patch, Size: 815 bytes --]

From a673aceab4eb0bcfa4e6ff34c02adc453383805b Mon Sep 17 00:00:00 2001
From: Anders Waldenborg <anders@0x63.nu>
Date: Sun, 23 Apr 2017 21:15:46 +0200
Subject: [PATCH] Fix memory leak of cwd string in emacsclient

* lib-src/emacsclient.c (main): emacsclient retrieves the current
  working directory using get_current_dir_name which returns a newly
  allocated string. Make sure this string is freed before exiting.
---
 lib-src/emacsclient.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index c22b308..3ffb9ea 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -1901,6 +1901,8 @@ main (int argc, char **argv)
   if (rl < 0)
     exit_status = EXIT_FAILURE;
 
+  free (cwd);
+
   CLOSE_SOCKET (emacs_socket);
   return exit_status;
 }
-- 
2.7.4

bug#26628: [PATCH] Fix memory leak of cwd string in emacsclient

[Andreas Schwab]

On Apr 23 2017, Anders Waldenborg <anders@0x63.nu> wrote:

> * lib-src/emacsclient.c (main): emacsclient retrieves the current
>   working directory using get_current_dir_name which returns a newly
>   allocated string. Make sure this string is freed before exiting.

There is no need to free it since the process exists right away anyway.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#26628: [PATCH] Fix memory leak of cwd string in emacsclient

[Anders Waldenborg]

On Mon, Apr 24, 2017 at 11:23:26AM +0200, Andreas Schwab wrote:
> > * lib-src/emacsclient.c (main): emacsclient retrieves the current
> >   working directory using get_current_dir_name which returns a newly
> >   allocated string. Make sure this string is freed before exiting.
> 
> There is no need to free it since the process exists right away anyway.

Yes. Unless you compile with -fsanitize=address and this leak makes
emacsclient mostly unusable as the asan leak checker will change the
return code to non-zero (yes I'm aware that I can set
ASAN_OPTIONS=detect_leaks=0 in the environment).

 anders

bug#26628: [PATCH] Fix memory leak of cwd string in emacsclient

[npostavs]

tags 26628 fixed
close 26628 26.1
quit

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Apr 23 2017, Anders Waldenborg <anders@0x63.nu> wrote:
>
>> * lib-src/emacsclient.c (main): emacsclient retrieves the current
>>   working directory using get_current_dir_name which returns a newly
>>   allocated string. Make sure this string is freed before exiting.
>
> There is no need to free it since the process exists right away anyway.

On the other hand, it doesn't really hurt to free it either, pushed to
master [1: c221f1466e].

[1: c221f1466e]: 2017-05-31 22:58:30 -0400
  Fix memory leak of cwd string in emacsclient (Bug#26628)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c221f1466ed7e0f11f142d9cb3c0247b10e511c6

bug#26628: [PATCH] Fix memory leak of cwd string in emacsclient

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

Generally speaking we don't bother freeing storage just before program exit 
merely to pacify AddressSanitizer, as that makes the program less efficient and 
is contrary to the goal of leak checking which is to increase efficiency. Here, 
though, we can free storage earlier, and this might have a point since the 
storage can get reused. So I installed the attached further patch.

> I'm aware that I can set ASAN_OPTIONS=detect_leaks=0 in the environment

It might not hurt to do that, if only to prevent our hassling with false alarms.

[-- Attachment #2: 0001-Free-cwd-when-no-longer-needed.txt --]
[-- Type: text/plain, Size: 1395 bytes --]

From 877e808440d4bc2e62d6fb509defee91a3fdc895 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 31 May 2017 22:38:04 -0700
Subject: [PATCH] Free cwd when no longer needed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib-src/emacsclient.c (main): Don’t dally when freeing cwd.
---
 lib-src/emacsclient.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 3a0715f..8828b76 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -208,7 +208,7 @@ char *get_current_dir_name (void);
 /* Return the current working directory.  Returns NULL on errors.
    Any other returned value must be freed with free.  This is used
    only when get_current_dir_name is not defined on the system.  */
-char*
+char *
 get_current_dir_name (void)
 {
   char *buf;
@@ -1702,6 +1702,7 @@ main (int argc, char **argv)
   if (tramp_prefix)
     quote_argument (emacs_socket, tramp_prefix);
   quote_argument (emacs_socket, cwd);
+  free (cwd);
   send_to_emacs (emacs_socket, "/");
   send_to_emacs (emacs_socket, " ");
 
@@ -1945,8 +1946,6 @@ main (int argc, char **argv)
   if (rl < 0)
     exit_status = EXIT_FAILURE;
 
-  free (cwd);                   /* Keep leak checkers happy.  */
-
   CLOSE_SOCKET (emacs_socket);
   return exit_status;
 }
-- 
2.7.4