Print

Print


More nits/bugs:

After looking at malloc for a while we have been looking to see if the 
use of free/delete is consistent and found the following potential 
problems:

1. in src/XrdXr/XrdXrClientWorker.cc:
At line 304 we have
	credtype = malloc(4);
At line 368 we have
	delete credtype;
instead of free((void *) credtype);

2. In the same file at line 763 there is a delete of redirectHost but 
it was allocated with malloc at line 921.

3. In src/XrdXrootd/XrdXrootdConfig.cc:

There are two errors we detected in this code. The first is a file 
descriptor leak, and the second is a  memory leak. For example, in the 
method ConfigFn (line 251) we have the return value of a malloc being 
held by the local variable fbuff.  However, if there is an error during 
the read, they return without ever freeing fbuff. Likewise in several 
other places.  Here is Chris's corrected version of this. I have put a 
highlight comment on each change.

int XrdXrootdProtocol::ConfigFn(char *fn)
{
     struct stat buf;
     int fd, rsz, NoGo;
     char *fbuff;

     if ((fd = open(fn, O_RDONLY)) < 0)
        {eDest.Emsg("Config", errno, "open", fn); return 1;}
     if (fstat(fd, &buf) < 0)
     {
	 eDest.Emsg("Config", errno, "get size of", fn);
	 close(fd); // CDJ fix
	 return 1;
     }
     if (!(fbuff = (char *)malloc(buf.st_size+1)))
     {
	 eDest.Emsg("Config", errno, "get buffer for", fn);
	 close(fd);  // CDJ fix
	 return 1;
	}
     if ((rsz = read(fd, (void *)fbuff, buf.st_size)) < 0)
     {
	 eDest.Emsg("Config", errno, "read", fn);
  	 close(fd);     // CDJ fix
	 free(fbuff);   // CDJ fix
	 return 1;
      }
     close(fd);
     fbuff[rsz] = '\0';
     NoGo = (rsz ? ConfigIt(fbuff) : 0);
     free(fbuff);
     return NoGo;


Better than putting free calls everywhere would be a little class whose 
sole job is to manage a buffer, then all of this would be handled 
automatically when it went out of scope.

Regarding running valgrind, I think that is better done by one of the 
maintainers. It would take forever to document the errors, and we 
aren't in a position to fix them directly and send in the changes.

--
Gregory J. Sharp                   email: [log in to unmask]
Wilson Synchrotron Laboratory      url: 
http://www.lepp.cornell.edu/~gregor
Dryden Rd                          ph:  +1 607 255 4882
Ithaca, NY 14853                   fax: +1 607 255 8062