ratpoison-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[RP] [PATCH] Cleaned up some cmdret, sbuf, char * usage; eliminating ass


From: Kipling Inscore
Subject: [RP] [PATCH] Cleaned up some cmdret, sbuf, char * usage; eliminating associated memory leaks
Date: Tue, 30 Mar 2010 20:47:25 -0700

In response to the observation of C. Coutinho (below), I decided to fix some 
memory leaks in actions.c

On Tue, Mar 30, 2010 at 15:27, C. Coutinho <address@hidden> wrote:
>
> Hi,
>
> On my machine, ratpoison leaks memory when executing the command 'info'.
>
> You can see this in action by launching on the command line:
>
>  while true; do ratpoison -c info; done
>
> Then, if you launch 'top' and look at the memory consumption of
> ratpoison, you can see that it always grows.

I found (by looking in the source) and verified with this method that
unmanage (if there are an unmanaged window names)
readkey (given the name of an unknown keymap)
help
also leak.
It looks like exec completions should leak but I wasn't able see any increase 
in memory usage after trying a few completions.

>
> I looked at the source code but I don't really know the C programming
> language and how to track memory leak so I'm sorry I'm only reporting a
> bug without submitting a patch.
>
> I'm using ratpoison 1.4.5 on archlinux but it's the same if ratpoison is
> compiled from the git repository.

I cloned the git today; head at
commit f5246556adf56ae470a1e898f27ca0fead7d0f7b
Author: Shawn Betts <Shawn Betts address@hidden>
Date:   Sun Jan 31 11:30:45 2010 -0800

    change error message for gdelete to reduce confusion.

---
 src/actions.c |   35 ++++++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/actions.c b/src/actions.c
index bda8002..90429a6 100644
--- a/src/actions.c
+++ b/src/actions.c
@@ -1041,7 +1041,12 @@ cmdret *
 cmd_unmanage (int interactive, struct cmdarg **args)
 {
   if (args[0] == NULL && !interactive)
-    return cmdret_new (RET_SUCCESS, "%s", list_unmanaged_windows());
+    {
+      char *s = list_unmanaged_windows();
+      cmdret *ret = cmdret_new (RET_SUCCESS, "%s", s);
+      free (s);
+      return ret;
+    }
 
   if (args[0])
     add_unmanaged_window(ARG_STRING(0));
@@ -1522,7 +1527,11 @@ read_keymap (struct argspec *spec, struct sbuf *s, 
struct cmdarg **arg)
       rp_keymap *map;
       map = find_keymap (input);
       if (map == NULL)
-        return cmdret_new (RET_FAILURE, "unknown keymap '%s'", input);
+        {
+          cmdret *ret = cmdret_new (RET_FAILURE, "unknown keymap '%s'", input);
+          free (input);
+          return ret;
+        }
       *arg = xmalloc (sizeof(struct cmdarg));
       (*arg)->type = spec->type;
       (*arg)->arg.keymap = map;
@@ -1693,6 +1702,8 @@ exec_completions (char *str)
         }
     }
 
+  sbuf_free (line);
+
   free (partial);
   pclose (file);
 
@@ -3468,8 +3479,8 @@ cmd_help (int interactive, struct cmdarg **args)
     {
       struct sbuf *help_list;
       char *keysym_name;
-      char *tmp;
       int i;
+      cmdret *ret;
 
       help_list = sbuf_new (0);
 
@@ -3484,10 +3495,9 @@ cmd_help (int interactive, struct cmdarg **args)
             sbuf_concat (help_list, "\n");
         }
 
-      tmp = sbuf_get (help_list);
-      free (help_list);
-
-      return cmdret_new (RET_SUCCESS, "%s", tmp);
+      ret = cmdret_new (RET_SUCCESS, "%s", sbuf_get (help_list));
+      sbuf_free (help_list);
+      return ret;
     }
 
   return cmdret_new (RET_SUCCESS, NULL);
@@ -4186,7 +4196,6 @@ cmd_unsetenv (int interactive UNUSED, struct cmdarg 
**args)
   s = sbuf_new(0);
   sbuf_copy (s, ARG_STRING(0));
   sbuf_concat (s, "=");
-/*   str = sbuf_free_struct (s); */
   putenv (sbuf_get(s));
   sbuf_free (s);
   return cmdret_new (RET_SUCCESS, NULL);
@@ -4198,7 +4207,6 @@ cmdret *
 cmd_info (int interactive UNUSED, struct cmdarg **args)
 {
   struct sbuf *buf;
-  char *tmp;
   if (current_window() != NULL)
     {
       rp_window *win = current_window();
@@ -4210,6 +4218,7 @@ cmd_info (int interactive UNUSED, struct cmdarg **args)
       if (win_elem)
         {
           char *s;
+          cmdret *ret;
 
           if (args[0] == NULL)
             s = defaults.info_fmt;
@@ -4217,8 +4226,9 @@ cmd_info (int interactive UNUSED, struct cmdarg **args)
             s = ARG_STRING(0);
           buf = sbuf_new (0);
           format_string (s, win_elem, buf);
-          tmp = sbuf_free_struct (buf);
-          return cmdret_new (RET_SUCCESS, "%s", tmp);
+          ret = cmdret_new (RET_SUCCESS, "%s", sbuf_get (buf));
+          sbuf_free (buf);
+          return ret;
         }
     }
 
@@ -4831,8 +4841,7 @@ fdump (rp_screen *screen)
       free (t);
     }
 
-  tmp = sbuf_get (s);
-  free (s);
+  tmp = sbuf_free_struct (s);
   return tmp;
 }
 
-- 
1.6.3.3





reply via email to

[Prev in Thread] Current Thread [Next in Thread]