Hi,

I have tried the cppcheck static code analyzer on apr trunk and it 
found some errors that may be worth fixing. For some, I have attached 
a patch. These two I think are false positives/intentional:

[./dso/win32/dso.c:133]: (error) Uninitialized variable: rv
[./misc/unix/otherchild.c:85]: (error) Possible null pointer 
dereference: cur

I haven't looked at the rest:

[./build/aplibtool.c:379]: (error) Memory leak: newarg
[./build/aplibtool.c:637]: (error) Resource leak: dir
[./build/jlibtool.c:978]: (error) Memory leak: path
[./build/jlibtool.c:1775]: (error) Memory leak: cctemp
[./build/jlibtool.c:1961]: (error) Resource leak: dir
[./build/jlibtool.c:1653]: (error) Data is allocated but not 
initialized: newpath
[./test/testbuckets.c:108]: (error) Undefined behaviour: msg is used 
wrong in call to sprintf or snprintf. Quote: If copying takes place 
between objects that overlap as a result of a call to sprintf() or 
snprintf(), the results are undefined.

Given the relatively low rate of false positives, I think cppcheck is 
a nice tool.

Cheers,
Stefan
Index: memory/unix/apr_pools.c
===================================================================
--- memory/unix/apr_pools.c	(Revision 930965)
+++ memory/unix/apr_pools.c	(Arbeitskopie)
@@ -1427,6 +1427,7 @@
     node = pool->nodes;
     if (node == NULL || node->index == 64) {
         if ((node = malloc(SIZEOF_DEBUG_NODE_T)) == NULL) {
+            free(mem);
             if (pool->abort_fn)
                 pool->abort_fn(APR_ENOMEM);
 
Index: dbd/apr_dbd_oracle.c
===================================================================
--- dbd/apr_dbd_oracle.c	(Revision 930965)
+++ dbd/apr_dbd_oracle.c	(Arbeitskopie)
@@ -1758,8 +1758,8 @@
 {
     int ret = 1;             /* no transaction is an error cond */
     sword status;
-    apr_dbd_t *handle = trans->handle;
     if (trans) {
+        apr_dbd_t *handle = trans->handle;
         switch (trans->status) {
         case TRANS_NONE:     /* No trans is an error here */
             status = OCI_ERROR;
Index: threadproc/beos/proc.c
===================================================================
--- threadproc/beos/proc.c	(Revision 930965)
+++ threadproc/beos/proc.c	(Arbeitskopie)
@@ -362,8 +362,9 @@
                     == APR_SUCCESS)
                 rv = apr_file_inherit_set(attr->child_in);
         }
+    }
 
-    if (parent_in != NULL && rv == APR_SUCCESS) {
+    if (parent_in != NULL && rv == APR_SUCCESS)
         rv = apr_file_dup(&attr->parent_in, parent_in, attr->pool);
 
     return rv;
@@ -391,7 +392,7 @@
         }
     }
   
-    if (parent_out != NULL && rv == APR_SUCCESS) {
+    if (parent_out != NULL && rv == APR_SUCCESS)
         rv = apr_file_dup(&attr->parent_out, parent_out, attr->pool);
 
     return rv;
@@ -419,7 +420,7 @@
         }
     }
   
-    if (parent_err != NULL && rv == APR_SUCCESS) {
+    if (parent_err != NULL && rv == APR_SUCCESS)
         rv = apr_file_dup(&attr->parent_err, parent_err, attr->pool);
 
     return rv;
Index: file_io/unix/open.c
===================================================================
--- file_io/unix/open.c	(Revision 930965)
+++ file_io/unix/open.c	(Arbeitskopie)
@@ -180,13 +180,17 @@
     if (!(flag & APR_FOPEN_NOCLEANUP)) {
         int flags;
 
-        if ((flags = fcntl(fd, F_GETFD)) == -1)
+        if ((flags = fcntl(fd, F_GETFD)) == -1) {
+            close(fd);
             return errno;
+        }
 
         if ((flags & FD_CLOEXEC) == 0) {
             flags |= FD_CLOEXEC;
-            if (fcntl(fd, F_SETFD, flags) == -1)
+            if (fcntl(fd, F_SETFD, flags) == -1) {
+                close(fd);
                 return errno;
+            }
         }
     }
 

Reply via email to