On Mon, 19 Sep 2005, Viktor Griph wrote:
On Sun, 18 Sep 2005, Mikhael Goikhman wrote: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)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.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
After a night worth of sleep I realized that the first approach really does look bad. The second approch is the way to go. I've refined to code a little, and also added the code to make it expand names even if the final expansion fails. If it's too mush conflicts with the changes you've made, then just apply your changes and I'll update my patch against the new CVS version.
Attached it the patch or the expand.c file (other files are the same as the first patch), both vs the current CVS and vs the first patch.
/Viktor
Index: fvwm/expand.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/expand.c,v
retrieving revision 1.17
diff -u -r1.17 expand.c
--- fvwm/expand.c 3 Jun 2004 12:43:24 -0000 1.17
+++ fvwm/expand.c 19 Sep 2005 06:25:48 -0000
@@ -699,7 +699,20 @@
break;
default:
/* unknown variable - try to find it in the environment */
- string = getenv(var_name);
+ if (!(string = getenv(var_name)))
+ {
+ /* Replace it with unexpanded variable. This is needed
+ * since var_name might have been expanded */
+ l = strlen(var_name)+3;
+ if (output)
+ {
+ strcpy(output, "$[");
+ strcpy(output+2, var_name);
+ output[l-1] = ']';
+ output[l] = 0;
+ }
+ return l;
+ }
}
if (is_numeric)
{
@@ -723,7 +736,8 @@
cond_rc_t *cond_rc, const exec_context_t *exc)
{
int l, i, l2, n, k, j, m;
- int xlen;
+ int xlen, xlevel;
+ Bool name_has_dollar;
char *out;
char *var;
const char *string = NULL;
@@ -755,9 +769,27 @@
/* extended variables */
var = &input[i+2];
m = i + 2;
- while (m < l && input[m] != ']' && input[m])
+ xlevel = 1;
+ name_has_dollar = False;
+ while (m < l && xlevel && input[m])
{
- m++;
+ /* handle nested variables */
+ if (input[m] == ']')
+ {
+ xlevel--;
+ }
+ else if (input[m] == '[')
+ {
+ xlevel++;
+ }
+ else if (input[m] == '$')
+ {
+ name_has_dollar = True;
+ }
+ if (xlevel)
+ {
+ m++;
+ }
}
if (input[m] == ']')
{
@@ -766,9 +798,23 @@
k = strlen(var);
if (!addto)
{
+ if (name_has_dollar)
+ {
+ var = expand_vars(
+ var,
+ arguments,
+ addto,
+ ismod,
+ cond_rc,
+ exc);
+ }
xlen = expand_vars_extended(
var, NULL, cond_rc,
exc);
+ if (name_has_dollar)
+ {
+ free(var);
+ }
if (xlen >= 0)
{
l2 += xlen - (k + 2);
@@ -890,17 +936,49 @@
}
var = &input[i+2];
m = i + 2;
- while (m < l && input[m] != ']' && input[m])
+ xlevel = 1;
+ name_has_dollar = False;
+ while (m < l && xlevel && input[m])
{
- m++;
+ /* handle nested variables */
+ if (input[m] == ']')
+ {
+ xlevel--;
+ }
+ else if (input[m] == '[')
+ {
+ xlevel++;
+ }
+ else if (input[m] == '$')
+ {
+ name_has_dollar = True;
+ }
+ if (xlevel)
+ {
+ m++;
+ }
}
if (input[m] == ']')
{
input[m] = 0;
/* handle variable name */
k = strlen(var);
+ if (name_has_dollar)
+ {
+ var = expand_vars(
+ var,
+ arguments,
+ addto,
+ ismod,
+ cond_rc,
+ exc);
+ }
xlen = expand_vars_extended(
var, &out[j], cond_rc, exc);
+ if (name_has_dollar)
+ {
+ free(var);
+ }
input[m] = ']';
if (xlen >= 0)
{
--- ../fvwm-work/fvwm/expand.c 2005-09-19 01:24:06.000000000 +0200
+++ fvwm/expand.c 2005-09-19 08:24:00.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;
@@ -217,8 +217,6 @@
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)))
{
@@ -701,7 +699,20 @@
break;
default:
/* unknown variable - try to find it in the environment */
- string = getenv(var_name);
+ if (!(string = getenv(var_name)))
+ {
+ /* Replace it with unexpanded variable. This is needed
+ * since var_name might have been expanded */
+ l = strlen(var_name)+3;
+ if (output)
+ {
+ strcpy(output, "$[");
+ strcpy(output+2, var_name);
+ output[l-1] = ']';
+ output[l] = 0;
+ }
+ return l;
+ }
}
if (is_numeric)
{
@@ -726,6 +737,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 +770,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 +782,10 @@
{
xlevel++;
}
+ else if (input[m] == '$')
+ {
+ name_has_dollar = True;
+ }
if (xlevel)
{
m++;
@@ -781,10 +798,23 @@
k = strlen(var);
if (!addto)
{
+ if (name_has_dollar)
+ {
+ var = expand_vars(
+ var,
+ arguments,
+ addto,
+ ismod,
+ cond_rc,
+ exc);
+ }
xlen = expand_vars_extended(
- var, NULL, arguments,
- cond_rc,
+ var, NULL, cond_rc,
exc);
+ if (name_has_dollar)
+ {
+ free(var);
+ }
if (xlen >= 0)
{
l2 += xlen - (k + 2);
@@ -907,6 +937,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 +949,10 @@
{
xlevel++;
}
+ else if (input[m] == '$')
+ {
+ name_has_dollar = True;
+ }
if (xlevel)
{
m++;
@@ -928,9 +963,22 @@
input[m] = 0;
/* handle variable name */
k = strlen(var);
+ if (name_has_dollar)
+ {
+ var = expand_vars(
+ var,
+ arguments,
+ addto,
+ ismod,
+ cond_rc,
+ exc);
+ }
xlen = expand_vars_extended(
- var, &out[j], arguments,
- cond_rc, exc);
+ var, &out[j], cond_rc, exc);
+ if (name_has_dollar)
+ {
+ free(var);
+ }
input[m] = ']';
if (xlen >= 0)
{
