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 
> Help-cfengine@gnu.org 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);
 

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Help-cfengine mailing list
Help-cfengine@gnu.org
http://lists.gnu.org/mailman/listinfo/help-cfengine

Reply via email to