You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by sf...@apache.org on 2010/07/28 00:09:45 UTC

svn commit: r979891 - in /apr/apr/trunk: build/ dbd/ dbm/ file_io/netware/ file_io/os2/ file_io/unix/ file_io/win32/ memory/unix/ threadproc/beos/ threadproc/unix/

Author: sf
Date: Tue Jul 27 22:09:45 2010
New Revision: 979891

URL: http://svn.apache.org/viewvc?rev=979891&view=rev
Log:
Fix various issues found by cppcheck
- error handling issues
- use of uninitialized data
- null pointer dereference
- unused variables
- memory/fd leaks
- broken code in threadproc/beos/proc.c

Modified:
    apr/apr/trunk/build/aplibtool.c
    apr/apr/trunk/build/jlibtool.c
    apr/apr/trunk/dbd/apr_dbd_oracle.c
    apr/apr/trunk/dbm/apr_dbm_sdbm.c
    apr/apr/trunk/file_io/netware/pipe.c
    apr/apr/trunk/file_io/os2/open.c
    apr/apr/trunk/file_io/unix/open.c
    apr/apr/trunk/file_io/win32/open.c
    apr/apr/trunk/memory/unix/apr_pools.c
    apr/apr/trunk/threadproc/beos/proc.c
    apr/apr/trunk/threadproc/unix/proc.c
    apr/apr/trunk/threadproc/unix/procsup.c

Modified: apr/apr/trunk/build/aplibtool.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/build/aplibtool.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/build/aplibtool.c (original)
+++ apr/apr/trunk/build/aplibtool.c Tue Jul 27 22:09:45 2010
@@ -300,7 +300,6 @@ bool parse_input_file_name(char *arg, cm
 {
     char *ext = strrchr(arg, '.');
     char *name = strrchr(arg, '/');
-    int pathlen;
     char *newarg;
 
     if (!ext) {
@@ -321,8 +320,6 @@ bool parse_input_file_name(char *arg, cm
         name++;
     }
 
-    pathlen = name - arg;
-
     if (strcmp(ext, "lo") == 0) {
         newarg = (char *)malloc(strlen(arg) + 10);
         strcpy(newarg, arg);
@@ -362,7 +359,6 @@ bool parse_output_file_name(char *arg, c
     char *name = strrchr(arg, '/');
     char *ext = strrchr(arg, '.');
     char *newarg = NULL, *newext;
-    int pathlen;
 
     if (name == NULL) {
         name = strrchr(arg, '\\');
@@ -392,7 +388,6 @@ bool parse_output_file_name(char *arg, c
     }
 
     ext++;
-    pathlen = name - arg;
 
     if (strcmp(ext, "exe") == 0) {
         cmd_data->output_type = otProgram;

Modified: apr/apr/trunk/build/jlibtool.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/build/jlibtool.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/build/jlibtool.c (original)
+++ apr/apr/trunk/build/jlibtool.c Tue Jul 27 22:09:45 2010
@@ -1650,7 +1650,7 @@ const char* expand_path(const char *relp
 
     getcwd(foo, PATH_MAX-1);
     newpath = (char*)malloc(strlen(foo)+strlen(relpath)+2);
-    strcat(newpath, foo);
+    strcpy(newpath, foo);
     strcat(newpath, "/");
     strcat(newpath, relpath);
     return newpath;
@@ -1682,7 +1682,7 @@ void link_fixup(command_t *c)
             push_count_chars(c->shared_opts.normal, DYNAMIC_INSTALL_NAME);
 
             tmp = (char*)malloc(PATH_MAX);
-            strcat(tmp, c->install_path);
+            strcpy(tmp, c->install_path);
             strcat(tmp, strrchr(c->shared_name.normal, '/'));
             push_count_chars(c->shared_opts.normal, tmp);
 #endif

Modified: apr/apr/trunk/dbd/apr_dbd_oracle.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbd/apr_dbd_oracle.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/dbd/apr_dbd_oracle.c (original)
+++ apr/apr/trunk/dbd/apr_dbd_oracle.c Tue Jul 27 22:09:45 2010
@@ -1758,8 +1758,8 @@ static int dbd_oracle_end_transaction(ap
 {
     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;

Modified: apr/apr/trunk/dbm/apr_dbm_sdbm.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbm/apr_dbm_sdbm.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/dbm/apr_dbm_sdbm.c (original)
+++ apr/apr/trunk/dbm/apr_dbm_sdbm.c Tue Jul 27 22:09:45 2010
@@ -193,7 +193,7 @@ static apr_status_t vt_sdbm_nextkey(apr_
     pkey->dsize = rd.dsize;
 
     /* store any error info into DBM, and return a status code. */
-    return set_error(dbm, APR_SUCCESS);
+    return set_error(dbm, rv);
 }
 
 static void vt_sdbm_freedatum(apr_dbm_t *dbm, apr_datum_t data)

Modified: apr/apr/trunk/file_io/netware/pipe.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/netware/pipe.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/file_io/netware/pipe.c (original)
+++ apr/apr/trunk/file_io/netware/pipe.c Tue Jul 27 22:09:45 2010
@@ -26,7 +26,6 @@
 static apr_status_t pipeblock(apr_file_t *thepipe)
 {
 #ifdef USE_FLAGS
-    int				err;
 	unsigned long	flags;
 
 	if (fcntl(thepipe->filedes, F_GETFL, &flags) != -1)
@@ -49,7 +48,6 @@ static apr_status_t pipeblock(apr_file_t
 static apr_status_t pipenonblock(apr_file_t *thepipe)
 {
 #ifdef USE_FLAGS
-	int				err;
 	unsigned long	flags;
 
     errno = 0;
@@ -138,7 +136,6 @@ APR_DECLARE(apr_status_t) apr_os_pipe_pu
 APR_DECLARE(apr_status_t) apr_file_pipe_create(apr_file_t **in, apr_file_t **out, apr_pool_t *pool)
 {
 	int     	filedes[2];
-	int 		err;
 
     if (pipe(filedes) == -1) {
         return errno;

Modified: apr/apr/trunk/file_io/os2/open.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/os2/open.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/file_io/os2/open.c (original)
+++ apr/apr/trunk/file_io/os2/open.c Tue Jul 27 22:09:45 2010
@@ -148,7 +148,7 @@ APR_DECLARE(apr_status_t) apr_file_close
         file->mutex = NULL;
     }
 
-    return APR_SUCCESS;
+    return status;
 }
 
 

Modified: apr/apr/trunk/file_io/unix/open.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/open.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/file_io/unix/open.c (original)
+++ apr/apr/trunk/file_io/unix/open.c Tue Jul 27 22:09:45 2010
@@ -180,13 +180,17 @@ APR_DECLARE(apr_status_t) apr_file_open(
     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;
+            }
         }
     }
 

Modified: apr/apr/trunk/file_io/win32/open.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/open.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/file_io/win32/open.c (original)
+++ apr/apr/trunk/file_io/win32/open.c Tue Jul 27 22:09:45 2010
@@ -145,7 +145,6 @@ void *res_name_from_filename(const char 
         apr_wchar_t *wpre, *wfile, *ch;
         apr_size_t n = strlen(file) + 1;
         apr_size_t r, d;
-        apr_status_t rv;
 
         if (apr_os_level >= APR_WIN_2000) {
             if (global)
@@ -169,7 +168,7 @@ void *res_name_from_filename(const char 
         wfile = apr_palloc(pool, (r + n) * sizeof(apr_wchar_t));
         wcscpy(wfile, wpre);
         d = n;
-        if ((rv = apr_conv_utf8_to_ucs2(file, &n, wfile + r, &d))) {
+        if (apr_conv_utf8_to_ucs2(file, &n, wfile + r, &d)) {
             return NULL;
         }
         for (ch = wfile + r; *ch; ++ch) {
@@ -186,7 +185,6 @@ void *res_name_from_filename(const char 
         apr_size_t n = strlen(file) + 1;
 
 #if !APR_HAS_UNICODE_FS
-        apr_status_t rv;
         apr_size_t r, d;
         char *pre;
 

Modified: apr/apr/trunk/memory/unix/apr_pools.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/memory/unix/apr_pools.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/memory/unix/apr_pools.c (original)
+++ apr/apr/trunk/memory/unix/apr_pools.c Tue Jul 27 22:09:45 2010
@@ -1427,6 +1427,7 @@ static void *pool_alloc(apr_pool_t *pool
     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);
 

Modified: apr/apr/trunk/threadproc/beos/proc.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/beos/proc.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/beos/proc.c (original)
+++ apr/apr/trunk/threadproc/beos/proc.c Tue Jul 27 22:09:45 2010
@@ -362,8 +362,9 @@ APR_DECLARE(apr_status_t) apr_procattr_c
                     == 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 @@ APR_DECLARE(apr_status_t) apr_procattr_c
         }
     }
   
-    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 @@ APR_DECLARE(apr_status_t) apr_procattr_c
         }
     }
   
-    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;

Modified: apr/apr/trunk/threadproc/unix/proc.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/unix/proc.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/unix/proc.c (original)
+++ apr/apr/trunk/threadproc/unix/proc.c Tue Jul 27 22:09:45 2010
@@ -396,7 +396,6 @@ APR_DECLARE(apr_status_t) apr_proc_creat
         return errno;
     }
     else if (new->pid == 0) {
-        int status;
         /* child process */
 
         /*
@@ -478,7 +477,7 @@ APR_DECLARE(apr_status_t) apr_proc_creat
         }
         /* Only try to switch if we are running as root */
         if (attr->gid != -1 && !geteuid()) {
-            if ((status = setgid(attr->gid))) {
+            if (setgid(attr->gid)) {
                 if (attr->errfn) {
                     attr->errfn(pool, errno, "setting of group failed");
                 }
@@ -487,7 +486,7 @@ APR_DECLARE(apr_status_t) apr_proc_creat
         }
 
         if (attr->uid != -1 && !geteuid()) {
-            if ((status = setuid(attr->uid))) {
+            if (setuid(attr->uid)) {
                 if (attr->errfn) {
                     attr->errfn(pool, errno, "setting of user failed");
                 }
@@ -495,7 +494,7 @@ APR_DECLARE(apr_status_t) apr_proc_creat
             }
         }
 
-        if ((status = limit_proc(attr)) != APR_SUCCESS) {
+        if (limit_proc(attr) != APR_SUCCESS) {
             if (attr->errfn) {
                 attr->errfn(pool, errno, "setting of resource limits failed");
             }

Modified: apr/apr/trunk/threadproc/unix/procsup.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/unix/procsup.c?rev=979891&r1=979890&r2=979891&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/unix/procsup.c (original)
+++ apr/apr/trunk/threadproc/unix/procsup.c Tue Jul 27 22:09:45 2010
@@ -18,8 +18,6 @@
 
 APR_DECLARE(apr_status_t) apr_proc_detach(int daemonize)
 {
-    int x;
-
     if (chdir("/") == -1) {
         return errno;
     }
@@ -28,6 +26,8 @@ APR_DECLARE(apr_status_t) apr_proc_detac
     /* Don't detach for MPE because child processes can't survive the death of
      * the parent. */
     if (daemonize) {
+        int x;
+
         if ((x = fork()) > 0) {
             exit(0);
         }



Re: svn commit: r979891 - in /apr/apr/trunk: build/ dbd/ dbm/ file_io/netware/ file_io/os2/ file_io/unix/ file_io/win32/ memory/unix/ threadproc/beos/ threadproc/unix/

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Wed, 28 Jul 2010, Ruediger Pluem wrote:
> On 07/28/2010 12:09 AM, sf@apache.org wrote:
>> Author: sf
>> Date: Tue Jul 27 22:09:45 2010
>> New Revision: 979891

>> Modified: apr/apr/trunk/dbm/apr_dbm_sdbm.c
>> URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbm/apr_dbm_sdbm.c?rev=979891&r1=979890&r2=979891&view=diff
>> ==============================================================================
>> --- apr/apr/trunk/dbm/apr_dbm_sdbm.c (original)
>> +++ apr/apr/trunk/dbm/apr_dbm_sdbm.c Tue Jul 27 22:09:45 2010
>> @@ -193,7 +193,7 @@ static apr_status_t vt_sdbm_nextkey(apr_
>>      pkey->dsize = rd.dsize;
>>
>>      /* store any error info into DBM, and return a status code. */
>> -    return set_error(dbm, APR_SUCCESS);
>> +    return set_error(dbm, rv);
>
> This breaks the testdbm test of apr.

Sorry, reverted in r980197 for now.

This discards any error from getnext() in sdbm.c, including those from the 
chkpage consistency check. But ATM, I am not sure how to fix this 
correctly.

Re: svn commit: r979891 - in /apr/apr/trunk: build/ dbd/ dbm/ file_io/netware/ file_io/os2/ file_io/unix/ file_io/win32/ memory/unix/ threadproc/beos/ threadproc/unix/

Posted by Ruediger Pluem <rp...@apache.org>.

On 07/28/2010 12:09 AM, sf@apache.org wrote:
> Author: sf
> Date: Tue Jul 27 22:09:45 2010
> New Revision: 979891
> 
> URL: http://svn.apache.org/viewvc?rev=979891&view=rev
> Log:
> Fix various issues found by cppcheck
> - error handling issues
> - use of uninitialized data
> - null pointer dereference
> - unused variables
> - memory/fd leaks
> - broken code in threadproc/beos/proc.c
> 
> Modified:
>     apr/apr/trunk/build/aplibtool.c
>     apr/apr/trunk/build/jlibtool.c
>     apr/apr/trunk/dbd/apr_dbd_oracle.c
>     apr/apr/trunk/dbm/apr_dbm_sdbm.c
>     apr/apr/trunk/file_io/netware/pipe.c
>     apr/apr/trunk/file_io/os2/open.c
>     apr/apr/trunk/file_io/unix/open.c
>     apr/apr/trunk/file_io/win32/open.c
>     apr/apr/trunk/memory/unix/apr_pools.c
>     apr/apr/trunk/threadproc/beos/proc.c
>     apr/apr/trunk/threadproc/unix/proc.c
>     apr/apr/trunk/threadproc/unix/procsup.c
> 

> Modified: apr/apr/trunk/dbm/apr_dbm_sdbm.c
> URL: http://svn.apache.org/viewvc/apr/apr/trunk/dbm/apr_dbm_sdbm.c?rev=979891&r1=979890&r2=979891&view=diff
> ==============================================================================
> --- apr/apr/trunk/dbm/apr_dbm_sdbm.c (original)
> +++ apr/apr/trunk/dbm/apr_dbm_sdbm.c Tue Jul 27 22:09:45 2010
> @@ -193,7 +193,7 @@ static apr_status_t vt_sdbm_nextkey(apr_
>      pkey->dsize = rd.dsize;
>  
>      /* store any error info into DBM, and return a status code. */
> -    return set_error(dbm, APR_SUCCESS);
> +    return set_error(dbm, rv);

This breaks the testdbm test of apr.

Regards

RĂ¼diger