I agree this should exist. I have been burned before. And it is very
difficult to debug. All that aside there needs to be a parameter to
turn it on or off. Or a setting inside the code. I know for a fact I
have bits of code that use $() or ${} that expand in shell. (At least I
think I do)I would like to have: control: strict = ( on ) But that won't work in the code because the variable are expanded before the options are set. So I made a switch -r which turns strict checking off. Iadded it to both cfexecd and cfagent. I also made the output give the variable name and the line number. There are a few other goodies in the way of output in cfexecd. BTW: I think Mark is on holiday. Can some one test it? On Wed, 2005-07-13 at 00:33 -0700, Eric Sorenson wrote: > Tracker ID: [ 1236867 ] Failed variable expansion should be an error > > On Fri, 8 Jul 2005, Chip Seraphine wrote: > > > For about the one bajillionth time, I troubleshot a problem in my > > environment > > and it came back to some variable being undefined. Rather than put explicit > > tests (strcmps and whatnot) for every variable I ever set, I would be much > > happier if I could pass some sort of switch to cfengine that would cause it > > to > > simply fail with a useful error message whenever a variable went undefined > > rather than handling as a string that happens to begin with '$'. It is far > > safer and more secure to abort than to run commands with garbage inputs.... > > > > I'm quite comfortable using $(dollar) when I need to, so I can't think of a > > downside of tis... > > I found where this is happening, and it seems like it'd be easy to > croak on an undefined variable -- A Patch is attached. But I just made > this the default instead of adding YET ANOTHER getopt. This might be > too big of a change, if people are used to this silently succeeding (?!). > > On my test case, before the patch I ended up with a file named > /tmp/$(testvar) which, truly I can't imagine being the desired > behavior. It seems like fixing this would really be a good way to catch > typos and so forth. Does anybody on the list rely on the current behavior? > > Mark, any opinion? Should we croak on the use of an uninitialzed > variable? What was the original reasoning behind passing the variable > through as-is? (in the 'if varstring ...' section deleted in my patch) > > > > -- > - Eric Sorenson - N37 17.255 W121 55.738 - http://eric.explosive.net - > - Personal colo with a professional touch - http://www.explosive.net - > _______________________________________________ Help-cfengine mailing list > [email protected] http://lists.gnu.org/mailman/listinfo/help-cfengine -- Christian Pearce Perfect Order, Inc. http://www.sysnav.com http://www.perfectorder.com
--- globals.c.orig 2005-07-13 11:55:22.320402096 -0400
+++ globals.c 2005-07-13 11:02:31.581427688 -0400
@@ -445,6 +445,7 @@
{ "avoid",required_argument,0,'o'},
{ "query",required_argument,0,'Q'},
{ "csdb",no_argument,0,'W'},
+ { "no-varcheck",no_argument,0,'r'},
{ NULL,0,0,0 }
};
@@ -675,6 +676,7 @@
PRIVATE short PTRAVLINKS = false;
PRIVATE short DEADLINKS = true;
PRIVATE short DONTDO = false;
+ PRIVATE short STRICT = true;
PRIVATE short IFCONF = true;
PRIVATE short PARSEONLY = false;
PRIVATE short GOTMOUNTINFO = true;
--- cf.extern.h.orig 2005-07-13 11:55:36.698216336 -0400
+++ cf.extern.h 2005-07-13 09:33:52.126107816 -0400
@@ -386,6 +386,7 @@
extern short DEADLINKS;
extern short PTRAVLINKS;
extern short DONTDO;
+extern short STRICT;
extern short IFCONF;
extern short PARSEONLY;
extern short GOTMOUNTINFO;
--- cfagent.c.orig 2005-07-13 11:55:47.656550416 -0400
+++ cfagent.c 2005-07-13 11:00:15.702084488 -0400
@@ -1626,7 +1626,7 @@
int c;
-while ((c=getopt_long(argc,argv,"WbBzMgAbKqkhYHd:vlniIf:pPmcCtsSaeEVD:N:LwxXuUj:o:Z:Q:",OPTIONS,&optindex)) != EOF)
+while ((c=getopt_long(argc,argv,"rWbBzMgAbKqkhYHd:vlniIf:pPmcCtsSaeEVD:N:LwxXuUj:o:Z:Q:",OPTIONS,&optindex)) != EOF)
{
switch ((char) c)
{
@@ -1644,6 +1644,9 @@
case 'B': UPDATEONLY = true;
break;
+
+ case 'r': STRICT = false;
+ break;
case 'f':
strncpy(VINPUTFILE,optarg, CF_BUFSIZE-1);
--- varstring.c.orig 2005-07-13 11:55:57.831003664 -0400
+++ varstring.c 2005-07-13 11:51:07.282173800 -0400
@@ -330,7 +330,9 @@
{ char *sp,*env;
char varstring = false;
+ char open = false;
char currentitem[CF_EXPANDSIZE],temp[CF_BUFSIZE],name[CF_MAXVARSIZE];
+ char output[CF_BUFSIZE];
int len;
time_t tloc;
@@ -362,9 +364,11 @@
switch (*(sp+1))
{
case '(':
+ open = '(';
varstring = ')';
break;
case '{':
+ open = '{';
varstring = '}';
break;
default:
@@ -769,20 +773,21 @@
strcat(buffer,env);
Debug("Expansion gave (%s)\n",buffer);
- break;
- }
+ }
+ else /* GetMacroValue came back NULL, i.e. variable is undefined */
+ {
+ if(STRICT) {
+ snprintf(output,CF_BUFSIZE,"Attempt to use nonexistent variable $%c%s%c on line number %d",open,currentitem,varstring,LINENUMBER-1);
+ FatalError(output);
+ } else { /* Givem back the variable and git-r-done */
+ Debug("Currently non existent variable $%c%s%c)\n",open,currentitem,varstring);
+ snprintf(name,CF_MAXVARSIZE,"$%c%s%c",open,currentitem,varstring);
+ strcat(buffer,name);
- Debug("Currently non existent variable $(%s)\n",currentitem);
-
- if (varstring == '}')
- {
- snprintf(name,CF_MAXVARSIZE,"${%s}",currentitem);
- }
- else
- {
- snprintf(name,CF_MAXVARSIZE,"$(%s)",currentitem);
}
- strcat(buffer,name);
+
+ }
+
}
sp += strlen(currentitem);
--- cfexecd.c.orig 2005-07-13 11:56:09.723195776 -0400
+++ cfexecd.c 2005-07-13 11:39:18.964854400 -0400
@@ -78,6 +78,8 @@
{ "foreground",no_argument,0,'g'},
{ "parse-only",no_argument,0,'p'},
{ "ld-library-path",required_argument,0,'L'},
+ { "no-varcheck",no_argument,0,'r'},
+ { "no-lock",no_argument,0,'K'},
{ NULL,0,0,0 }
};
@@ -150,7 +152,7 @@
sprintf(VPREFIX, "cfexecd");
openlog(VPREFIX,LOG_PID|LOG_NOWAIT|LOG_ODELAY,LOG_DAEMON);
-while ((c=getopt_long(argc,argv,"L:d:vhpqFV1g",CFDOPTIONS,&optindex)) != EOF)
+while ((c=getopt_long(argc,argv,"KL:d:vhpqFV1gr",CFDOPTIONS,&optindex)) != EOF)
{
switch ((char) c)
{
@@ -182,9 +184,15 @@
case 'q': NOSPLAY = true;
break;
+
+ case 'K': IGNORELOCK = true;
+ break;
case 'p': PARSEONLY = true;
break;
+
+ case 'r': STRICT = false;
+ break;
case 'g': NO_FORK = true;
break;
@@ -604,6 +612,7 @@
{ FILE *pp;
char line[CF_BUFSIZE],filename[CF_BUFSIZE],*sp;
char cmd[CF_BUFSIZE];
+ char cmd_opts[CF_BUFSIZE];
int print;
time_t starttime = time(NULL);
FILE *fp;
@@ -622,13 +631,17 @@
/* Need to make sure we have LD_LIBRARY_PATH here or children will die */
if (NOSPLAY)
- {
- snprintf(cmd,CF_BUFSIZE-1,"%s/bin/cfagent -q -Dfrom_cfexecd",CFWORKDIR);
- }
-else
- {
- snprintf(cmd,CF_BUFSIZE-1,"%s/bin/cfagent -Dfrom_cfexecd",CFWORKDIR);
- }
+ strncat(cmd_opts,"-q ",3);
+
+if (!STRICT)
+ strncat(cmd_opts,"-r ",3);
+
+if (IGNORELOCK)
+ strncat(cmd_opts,"-K ",3);
+
+snprintf(cmd,CF_BUFSIZE-1,"%s/bin/cfagent %s -Dfrom_cfexecd",CFWORKDIR,cmd_opts);
+
+Debug("Executing command: %s",cmd);
timestamp(starttime, line, CF_BUFSIZE);
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Help-cfengine mailing list [email protected] http://lists.gnu.org/mailman/listinfo/help-cfengine
