bug-cfengine
[Top][All Lists]
Advanced

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

[patch 2.1.0p1] image.c: more struct stat initialisation fixes


From: Sergio . Gelato
Subject: [patch 2.1.0p1] image.c: more struct stat initialisation fixes
Date: Sat, 13 Dec 2003 22:41:16 +0100
User-agent: Mutt/1.3.28i

Following my earlier findings in RecursiveImage(), I decided to review
other uses of struct stat objects in image.c. The following patch
corrects two definite problems:
        * in CheckImage(), deststatbuf wasn't initialised before calling
                CheckCopiedFile()
        * in PurgeFiles(), the value of statbuf was being used even
                after a detected and logged lstat() failure

Furthermore, the patch adds explicit checks of the return status from
stat() and lstat() wherever they were previously missing. It may be that
success of these calls was guaranteed by their context, but that's the
sort of optimisation that makes the code unauditable; better play it
safe. All failures are logged at cferror: if the code didn't bother
to check for them, I can only assume they are not meant to happen.

Side comment: the dstat argument to CopyReg() could be dropped. The
calling code already relies on this fact by not initialising the
actual argument before the call, which raises alarm flags at code 
review time.

--- orig/src/image.c
+++ mod/src/image.c
@@ -463,7 +463,15 @@
 
    /* Now check any overrides */
 
-   
CheckCopiedFile(destdir,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+   if (stat(destdir,&deststatbuf) == -1)
+      {
+      snprintf(OUTPUT,bufsize*2,"Can't stat directory %s\n",destdir);
+      CfLog(cferror,OUTPUT,"stat");
+      }
+   else
+      {
+      
CheckCopiedFile(destdir,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+      }
    
    for (dirp = cfreaddir(dirh,ip); dirp != NULL; dirp = cfreaddir(dirh,ip))
       {
@@ -583,8 +591,7 @@
            snprintf(OUTPUT,bufsize*2,"Couldn't stat %s while 
purging\n",filename);
            CfLog(cfverbose,OUTPUT,"stat");
            }
-        
-        if (S_ISDIR(statbuf.st_mode))
+        else if (S_ISDIR(statbuf.st_mode))
            {
            struct Tidy tp;
            struct TidyPattern tpat;
@@ -722,8 +729,15 @@
       if (succeed)
         {
         ENFORCELINKS = enforcelinks;
-        lstat(destfile,&deststatbuf);
-        
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+        if (lstat(destfile,&deststatbuf) == -1)
+           {
+            snprintf(OUTPUT,bufsize*2,"Can't lstat %s\n",destfile);
+           CfLog(cferror,OUTPUT,"lstat");
+           }
+        else
+           {
+           
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+           }
         }
       
       return;
@@ -826,8 +840,15 @@
 
       if (CopyReg(sourcefile,destfile,sourcestatbuf,deststatbuf,ip))
          {
-        stat(destfile,&deststatbuf);
-        
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+        if (stat(destfile,&deststatbuf) == -1)
+           {
+           snprintf(OUTPUT,bufsize*2,"Can't stat %s\n",destfile);
+           CfLog(cferror,OUTPUT,"stat");
+           }
+        else
+           {
+           
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+           }
         AddMultipleClasses(ip->defines);
 
         for (ptr = VAUTODEFINE; ptr != NULL; ptr=ptr->next)
@@ -966,8 +987,15 @@
 
       if (succeed)
         {
-        lstat(destfile,&deststatbuf);
-        
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+        if (lstat(destfile,&deststatbuf) == -1)
+           {
+           snprintf(OUTPUT,bufsize*2,"Can't lstat %s\n",destfile);
+           CfLog(cferror,OUTPUT,"lstat");
+           }
+        else
+           {
+           
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+           }
         AddMultipleClasses(ip->defines);
         }
       }
@@ -1087,8 +1115,15 @@
 
          if (CopyReg(sourcefile,destfile,sourcestatbuf,deststatbuf,ip))
             {
-            stat(destfile,&deststatbuf);
-           
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+            if (stat(destfile,&deststatbuf) == -1)
+              {
+              snprintf(OUTPUT,bufsize*2,"Can't stat %s\n",destfile);
+              CfLog(cferror,OUTPUT,"stat");
+              }
+           else
+              {
+              
CheckCopiedFile(destfile,ip->plus,ip->minus,fixall,ip->uid,ip->gid,&deststatbuf,&sourcestatbuf,NULL,ip->acl_aliases);
+              }
 
            if (VSINGLECOPY != NULL)
               {
@@ -1550,7 +1585,12 @@
       }
    }
 
-stat(new,&dstat);
+if (stat(new,&dstat) == -1)
+   {
+   snprintf(OUTPUT,bufsize*2,"Can't stat new file %s\n",new);
+   CfLog(cferror,OUTPUT,"stat");
+   return false;
+   }
 
 if (dstat.st_size != sstat.st_size)
    {






reply via email to

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