On Sun, 18 Sep 2005, Mikhael Goikhman wrote:
On 18 Sep 2005 23:28:29 +0200, Viktor Griph wrote:
I just believe that I didn't freeded a varable returned by expand_vars,
thus having a memory leak. Could you check so that the var_name is
freed at the end of epand_vars_extended if it isn't already?
This is not trivial, since this function has 32 "return" statements. :)
I believe they may be mostly replaced with just "break", or similar.
A faster fix would be to create a wrapper function, like:
if (name_has_dollar)
{
var_name = expand_vars(var_name);
should_free = 1;
}
expand_vars_extended(var_name);
if (should_free)
{
free(var_name);
}
You may implement one or another fix when you have a time.
I've implemented another fix. While your suggested fix looks better, and
would drop the need for sending along *arguments[] to the
expand_vars_extended I had already started on the other fix when I
realized how you meant the fix to be made. Since your sugestion was easy
to do I also did that one. Choose the one that you think is best (I'm
starting to think the second one is, even if it doesn't subexpand
variables in variables not existing)
Both patches are against my old patch. The first kepst the flow of my old
patch, and aslo expands $[UNEXISTING_$[desk.n]_VAR] to
$[UNEXISTING_0_VAR], while the second moves the recusrion to expand_vars,
but does not expand $[UNEXISTING_$[desk.n]_VAR] at all (but it wouldn't
be too hard to add afterwards)
Select the patch you like most (and that is easiest to fit with your
modifications to expand.c)
/Viktor
--- fvwm/expand.c 2005-09-19 01:15:17.000000000 +0200
+++ ../fvwm/fvwm/expand.c 2005-09-19 01:17:08.000000000 +0200
@@ -31,6 +31,7 @@
#include "virtual.h"
#include "colorset.h"
#include "schedule.h"
+#include "expand.h"
#include "libs/FGettext.h"
#include "libs/charmap.h"
#include "libs/wcontext.h"
@@ -219,8 +220,15 @@
/* handle nested variables */
var_name=expand_vars(var_name,arguments,False,False,cond_rc,exc);
+ if ((i = GetTokenIndex(var_name, partial_function_vars, -1, &rest))
+ !=-1)
+ {
+ /* Found var_name - OK to free */
+ free(var_name);
+ }
+
/* allow partial matches for *.cs, gt, ... etc. variables */
- switch ((i = GetTokenIndex(var_name, partial_function_vars, -1, &rest)))
+ switch (i)
{
case VAR_FG_CS:
case VAR_BG_CS:
@@ -315,9 +323,12 @@
default:
break;
}
-
+ if ((i = GetTokenIndex(var_name, function_vars, 0, &rest)) != -1)
+ {
+ free(var_name);
+ }
/* only exact matches for all other variables */
- switch ((i = GetTokenIndex(var_name, function_vars, 0, &rest)))
+ switch (i)
{
case VAR_DESK_N:
is_numeric = True;
@@ -701,7 +712,24 @@
break;
default:
/* unknown variable - try to find it in the environment */
- string = getenv(var_name);
+ if (string = getenv(var_name))
+ {
+ free(var_name);
+ }
+ else
+ {
+ l = strlen(var_name)+3;
+ if (output)
+ {
+ strcpy(output, "$[");
+ strcpy(output+2, var_name);
+ output[l-1] = ']';
+ output[l] = 0;
+ }
+ free(var_name);
+ return l;
+ }
+ free(var_name);
}
if (is_numeric)
{
--- fvwm/expand.c.bug 2005-09-19 01:24:06.000000000 +0200
+++ fvwm/expand.c 2005-09-19 01:56:13.000000000 +0200
@@ -196,7 +196,7 @@
/* ---------------------------- local functions ---------------------------- */
static signed int expand_vars_extended(
- char *var_name, char *output,char *arguments[], cond_rc_t *cond_rc,
+ char *var_name, char *output, cond_rc_t *cond_rc,
const exec_context_t *exc)
{
char *s;
@@ -216,9 +216,7 @@
Bool is_x;
Window context_w = Scr.Root;
FvwmWindow *fw = exc->w.fw;
-
- /* handle nested variables */
- var_name=expand_vars(var_name,arguments,False,False,cond_rc,exc);
+
/* allow partial matches for *.cs, gt, ... etc. variables */
switch ((i = GetTokenIndex(var_name, partial_function_vars, -1, &rest)))
{
@@ -726,6 +724,7 @@
{
int l, i, l2, n, k, j, m;
int xlen, xlevel;
+ Bool name_has_dollar;
char *out;
char *var;
const char *string = NULL;
@@ -758,6 +757,7 @@
var = &input[i+2];
m = i + 2;
xlevel = 1;
+ name_has_dollar = False;
while (m < l && xlevel && input[m])
{
/* handle nested variables */
@@ -769,6 +769,10 @@
{
xlevel++;
}
+ else if (input[m] == '$')
+ {
+ name_has_dollar = True;
+ }
if (xlevel)
{
m++;
@@ -781,10 +785,29 @@
k = strlen(var);
if (!addto)
{
+ char *var_name;
+ if (name_has_dollar)
+ {
+ var_name = expand_vars(
+ var,
+ arguments,
+ addto,
+ ismod,
+ cond_rc,
+ exc);
+ }
+ else
+ {
+ var_name = var;
+ }
xlen = expand_vars_extended(
- var, NULL, arguments,
+ var_name, NULL,
cond_rc,
exc);
+ if (name_has_dollar)
+ {
+ free(var_name);
+ }
if (xlen >= 0)
{
l2 += xlen - (k + 2);
@@ -907,6 +930,7 @@
var = &input[i+2];
m = i + 2;
xlevel = 1;
+ name_has_dollar = False;
while (m < l && xlevel && input[m])
{
/* handle nested variables */
@@ -918,6 +942,10 @@
{
xlevel++;
}
+ else if (input[m] == '$')
+ {
+ name_has_dollar = True;
+ }
if (xlevel)
{
m++;
@@ -928,9 +956,28 @@
input[m] = 0;
/* handle variable name */
k = strlen(var);
+ char *var_name;
+ if (name_has_dollar)
+ {
+ var_name = expand_vars(
+ var,
+ arguments,
+ addto,
+ ismod,
+ cond_rc,
+ exc);
+ }
+ else
+ {
+ var_name = var;
+ }
xlen = expand_vars_extended(
- var, &out[j], arguments,
+ var_name, &out[j],
cond_rc, exc);
+ if (name_has_dollar)
+ {
+ free(var_name);
+ }
input[m] = ']';
if (xlen >= 0)
{
