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;
+ }
}
}