[Cvs-dev] CVS Coverity scan via NetBSD fixes

[Mark D. Baushke]

Hi Christos,

It seems that the Coverity run 31 took into
account a number of the patches I sent. 

At least one of the fixes (cid-1307) appears to
not have been incorporated as yet.

This is the fifth patch I have provided based on
the NetBSD sources
(CVSROOT=:ext:address@hidden/cvsroot).

The following patch is adapted from the one
committed to the STABLE branch on Savannah.nongnu.org
(CVSROOT=:pserver:address@hidden:/sources/cvs
 branch: cvs1-11-x-branch
 module: ccvs/src). 

I am still running a regression test of these
changes merged onto the FEATURE branch and I will
commit there when that is complete.

I would be much obliged if you would commit it to
the NetBSD sources.

I'll continue to work through the remaining items
as time permits.

        Thanks,
        -- Mark

Log message:
2006-05-25  Mark D. Baushke  <address@hidden>

        * add.c (add): Do not leak memory.
        [Fixes NetBSD coverity cid-2199.]

        * edit.c (onoff_fileproc): Do not leak memory.
        [Fixes NetBSD coverity cid-2201.]

        * edit.c (onoff_filesdoneproc): Do not leak memory.
        [Fixes NetBSD coverity cid-2202.]

        * lock.c (readers_exist): Add assert (lockdir).
        [Fixes NetBSD coverity cid-2411.]

        * rcs.c (RCS_findlock_or_tip): Do not leak memory.
        [Fixes NetBSD coverity cid-2198.]

        * rcs.c (RCS_getdate): Avoid possible NULL dereference.
        [Fixes NetBSD coverity cid-2412.]

        * server.c (serve_sticky): Do not leak file descriptors.
        [Fixes NetBSD coverity cid-2197.]

        * server.c (do_cvs_command): Do not leak memory.
        [Fixes NetBSD coverity cid-2204.]
        * server.c (do_cvs_command): Protect close (dev_null_fd) against
        invalid fd value in error_exit.
        [Fixes NetBSD coverity cid-1307.]

        * tag.c (add_to_val_tags): Do not leak memory. Be sure to
        clear_val_tags_lock before returning.
        [Fixes NetBSD coverity cid-2071.]

Index: add.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/add.c,v
retrieving revision 1.3
diff -u -p -r1.3 add.c
--- add.c       12 May 2006 15:33:17 -0000      1.3
+++ add.c       26 May 2006 16:32:36 -0000
@@ -161,11 +161,15 @@ add (argc, argv)
        int j;
 
        if (argc == 0)
+       {
            /* We snipped out all the arguments in the above sanity
               check.  We can just forget the whole thing (and we
               better, because if we fired up the server and passed it
               nothing, it would spit back a usage message).  */
+           if (options)
+               free (options);
            return err;
+       }
 
        start_server ();
        ign_setup ();
Index: edit.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/edit.c,v
retrieving revision 1.2
diff -u -p -r1.2 edit.c
--- edit.c      4 Feb 2006 16:29:56 -0000       1.2
+++ edit.c      26 May 2006 16:32:36 -0000
@@ -32,8 +32,10 @@ onoff_fileproc (callerdat, finfo)
     void *callerdat;
     struct file_info *finfo;
 {
-    fileattr_get0 (finfo->file, "_watched");
+    char *watched = fileattr_get0 (finfo->file, "_watched");
     fileattr_set (finfo->file, "_watched", turning_on ? "" : NULL);
+    if (watched != NULL)
+       free (watched);
     return 0;
 }
 
@@ -52,8 +54,10 @@ onoff_filesdoneproc (callerdat, err, rep
 {
     if (setting_default)
     {
-       fileattr_get0 (NULL, "_watched");
+       char *watched = fileattr_get0 (NULL, "_watched");
        fileattr_set (NULL, "_watched", turning_on ? "" : NULL);
+       if (watched != NULL)
+           free (watched);
     }
     return err;
 }
Index: lock.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/lock.c,v
retrieving revision 1.2
diff -u -p -r1.2 lock.c
--- lock.c      4 Feb 2006 16:29:56 -0000       1.2
+++ lock.c      26 May 2006 16:32:36 -0000
@@ -718,6 +718,9 @@ readers_exist (repository)
 #endif
 
     lockdir = lock_name (repository, "");
+
+    assert (lockdir != NULL);
+
     lockdir[strlen (lockdir) - 1] = '\0';   /* remove trailing slash */
 
     do {
Index: rcs.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/rcs.c,v
retrieving revision 1.3
diff -u -p -r1.3 rcs.c
--- rcs.c       12 May 2006 15:33:18 -0000      1.3
+++ rcs.c       26 May 2006 16:32:36 -0000
@@ -3050,6 +3050,8 @@ RCS_getdate (rcs, date, force_tag_match)
        {
            char *date_1_1 = vers->date;
 
+           assert (p->data != NULL);
+
            vers = p->data;
            if (RCS_datecmp (vers->date, date_1_1) != 0)
                return xstrdup ("1.1");
@@ -4758,6 +4760,7 @@ RCS_findlock_or_tip (rcs)
     char *user = getcaller();
     Node *lock, *p;
     List *locklist;
+    char *defaultrev = NULL;
 
     /* Find unique delta locked by caller. This code is very similar
        to the code in RCS_unlock -- perhaps it could be abstracted
@@ -4803,7 +4806,10 @@ RCS_findlock_or_tip (rcs)
        those error checks are to make users lock before a checkin, and we do
        that in other ways if at all anyway (e.g. rcslock.pl).  */
 
-    p = findnode (rcs->versions, RCS_getbranch (rcs, rcs->branch, 0));
+    defaultrev = RCS_getbranch (rcs, rcs->branch, 0);
+    p = findnode (rcs->versions, defaultrev);
+    if (defaultrev != NULL)
+       free (defaultrev);
     if (!p)
     {
        error (0, 0, "RCS file `%s' does not contain its default revision.",
Index: server.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/server.c,v
retrieving revision 1.5
diff -u -p -r1.5 server.c
--- server.c    12 May 2006 15:33:18 -0000      1.5
+++ server.c    26 May 2006 16:32:38 -0000
@@ -1238,6 +1238,7 @@ serve_sticky (arg)
        if (alloc_pending (80 + strlen (CVSADM_TAG)))
            sprintf (pending_error_text, "E cannot write to %s", CVSADM_TAG);
        pending_error = save_errno;
+       (void) fclose (f);
        return;
     }
     if (fclose (f) == EOF)
@@ -2952,9 +2953,10 @@ error  \n");
 
     /* OK, sit around getting all the input from the child.  */
     {
-       struct buffer *stdoutbuf;
-       struct buffer *stderrbuf;
-       struct buffer *protocol_inbuf;
+       struct buffer *stdoutbuf = NULL;
+       struct buffer *stderrbuf = NULL;
+       struct buffer *protocol_inbuf = NULL;
+       int err_exit = 0;
        /* Number of file descriptors to check in select ().  */
        int num_to_check;
        int count_needed = 1;
@@ -3007,7 +3009,8 @@ error  \n");
        {
            buf_output0 (buf_to_net, "E close failed\n");
            print_error (errno);
-           goto error_exit;
+           err_exit = 1;
+           goto child_finish;
        }
        stdout_pipe[1] = -1;
 
@@ -3015,7 +3018,8 @@ error  \n");
        {
            buf_output0 (buf_to_net, "E close failed\n");
            print_error (errno);
-           goto error_exit;
+           err_exit = 1;
+           goto child_finish;
        }
        stderr_pipe[1] = -1;
 
@@ -3023,7 +3027,8 @@ error  \n");
        {
            buf_output0 (buf_to_net, "E close failed\n");
            print_error (errno);
-           goto error_exit;
+           err_exit = 1;
+           goto child_finish;
        }
        protocol_pipe[1] = -1;
 
@@ -3032,7 +3037,8 @@ error  \n");
        {
            buf_output0 (buf_to_net, "E close failed\n");
            print_error (errno);
-           goto error_exit;
+           err_exit = 1;
+           goto child_finish;
        }
        flowcontrol_pipe[0] = -1;
 #endif /* SERVER_FLOWCONTROL */
@@ -3041,7 +3047,9 @@ error  \n");
        {
            buf_output0 (buf_to_net, "E close failed\n");
            print_error (errno);
-           goto error_exit;
+           dev_null_fd = -1;   /* Do not try to close it again. */
+           err_exit = 1;
+           goto child_finish;
        }
        dev_null_fd = -1;
 
@@ -3128,7 +3136,8 @@ error  \n");
                {
                    buf_output0 (buf_to_net, "E select failed\n");
                    print_error (errno);
-                   goto error_exit;
+                   err_exit = 1;
+                   goto child_finish;
                }
            } while (numfds < 0);
 
@@ -3161,7 +3170,8 @@ error  \n");
                {
                    buf_output0 (buf_to_net, "E buf_input_data failed\n");
                    print_error (status);
-                   goto error_exit;
+                   err_exit = 1;
+                   goto child_finish;
                }
 
                /*
@@ -3235,7 +3245,8 @@ error  \n");
                {
                    buf_output0 (buf_to_net, "E buf_input_data failed\n");
                    print_error (status);
-                   goto error_exit;
+                   err_exit = 1;
+                   goto child_finish;
                }
 
                /* What should we do with errors?  syslog() them?  */
@@ -3260,7 +3271,8 @@ error  \n");
                {
                    buf_output0 (buf_to_net, "E buf_input_data failed\n");
                    print_error (status);
-                   goto error_exit;
+                   err_exit = 1;
+                   goto child_finish;
                }
 
                /* What should we do with errors?  syslog() them?  */
@@ -3340,21 +3352,33 @@ E CVS locks may need cleaning up.\n");
                command_pid = -1;
        }
 
+      child_finish:
        /*
         * OK, we've waited for the child.  By now all CVS locks are free
         * and it's OK to block on the network.
         */
        set_block (buf_to_net);
        buf_flush (buf_to_net, 1);
-       buf_shutdown (protocol_inbuf);
-       buf_free (protocol_inbuf);
-       protocol_inbuf = NULL;
-       buf_shutdown (stderrbuf);
-       buf_free (stderrbuf);
-       stderrbuf = NULL;
-       buf_shutdown (stdoutbuf);
-       buf_free (stdoutbuf);
-       stdoutbuf = NULL;
+       if (protocol_inbuf)
+       {
+           buf_shutdown (protocol_inbuf);
+           buf_free (protocol_inbuf);
+           protocol_inbuf = NULL;
+       }
+       if (stderrbuf)
+       {
+           buf_shutdown (stderrbuf);
+           buf_free (stderrbuf);
+           stderrbuf = NULL;
+       }
+       if (stdoutbuf)
+       {
+           buf_shutdown (stdoutbuf);
+           buf_free (stdoutbuf);
+           stdoutbuf = NULL;
+       }
+       if (err_exit)
+           goto error_exit;
     }
 
     if (errs)
@@ -3378,7 +3402,8 @@ E CVS locks may need cleaning up.\n");
            command_pid = -1;
     }
 
-    close (dev_null_fd);
+    if (dev_null_fd >= 0)
+       close (dev_null_fd);
     close (protocol_pipe[0]);
     close (protocol_pipe[1]);
     close (stderr_pipe[0]);
@@ -3706,6 +3731,10 @@ server_checked_in (file, update_dir, rep
     const char *update_dir;
     const char *repository;
 {
+    assert (file);
+    assert (update_dir);
+    assert (repository);
+
     if (noexec)
        return;
     if (scratched_file != NULL && entries_line == NULL)
Index: tag.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/tag.c,v
retrieving revision 1.2
diff -u -p -r1.2 tag.c
--- tag.c       4 Feb 2006 16:29:56 -0000       1.2
+++ tag.c       26 May 2006 16:32:38 -0000
@@ -1275,7 +1275,13 @@ add_to_val_tags (name)
     val_tags_lock (current_parsed_root->directory);
 
     /* Check for presence again since we have a lock now.  */
-    if (is_in_val_tags (&db, name)) return;
+    if (is_in_val_tags (&db, name))
+    {
+       clear_val_tags_lock ();
+       if (db)
+           dbm_close (db);
+       return;
+    }
 
     /* Casting out const should be safe here - input datums are not
      * written to by the myndbm functions.