Re: [Cvs-dev] Re: Result of CVS Coverity scan, via NetBSD

[Mark D. Baushke]

Hi Christos,

Here is the fourth installment in our series of fixes to NetBSD xcvs.

[Should I just ignore the ChangeLog patches?
You don't seem to be applying them...]

I have applied this patch to CVS STABLE. I am doing a check before
installing the same patch into the CVS FEATURE branch.

        Enjoy!
        -- Mark

Log message:
* log.c (log_expand_revlist): Add assert (r->first). It should
only be possible for both r->first == NULL && r->last == NULL
which would have been handled.
[Fixes NetBSD coverity cid-1063.]

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

Index: ChangeLog
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/ChangeLog,v
retrieving revision 1.2
diff -u -p -r1.2 ChangeLog
--- ChangeLog   4 May 2006 15:39:34 -0000       1.2
+++ ChangeLog   12 May 2006 21:38:24 -0000
@@ -1,18 +1,76 @@
+2006-05-12  Mark D. Baushke  <address@hidden>
+
+       * log.c (log_expand_revlist): Add assert (r->first). It should
+       only be possible for both r->first == NULL && r->last == NULL
+       which would have been handled.
+       [Fixes NetBSD coverity cid-1063.]
+
+       * server.c (do_cvs_command): Protect close (dev_null_fd) against
+       invalid fd value in error_exit.
+       [Fixes NetBSD coverity cid-1307.]
+       
+       * rcs.c (RCS_isdead): Assert that the first argument is not NULL.
+       [Fixes NetBSD coverity cid-1058.]
+
+       * commit.c (checkaddfile): Do not dereference NULL on call to
+       error().
+       [Fixes NetBSD coverity cid-1061.]
+
+       * log.c (cvslog): Assert p->start && p->end instead of masking the
+       problem.
+       * server.c (server_updated): Assert findnode_fn results instead of
+       masking the problem.
+       
+       * add.c (add_directory): Revert previous change. The xstrdup()
+       function already deals a NULL argument.
+       * client.c (handle_mt): Ditto.
+       * entries.c (Entnode_Create): Ditto.
+       (Entries_Open): Ditto.
+       * logmsg.c (fmt_proc): Ditto.
+       * vers_ts.c (Version_TS): Ditto.
+       
+2006-05-11  Mark D. Baushke  <address@hidden>
+
+       * add.c (add_directory): Protect tag from NULL dereference.
+       [Fixes NetBSD cid-1054.]
+
+       * client.c (handle_mt): Deal with missing text argument.
+       [Fixes NetBSD cid-924.]
+
+       * entries.c (Entnode_Create): Protect date, tag and ts_conflict
+       from possible NULL dereference.
+       [Fixes NetBSD coverity cid-994, cid-995, cid-1055, cid-1057.]
+
+       * entries.c (Entries_Open): Protect dirtag and dirdate from
+       possible NULL dereference.
+       [Fixes NetBSD coverity cid-996.]
+
+       * log.c (cvslog): Validate start and end args to
+       date_to_internet().
+       [Fixes NetBSD coverity cid-2427 and cid-2428.]
+
+       * logmsg.c (fmt_proc): Protect li->tag from NULL dereference.
+       [Fixes NetBSD coverity cid-997.]
+
+       * vers_ts.c (Version_TS): Protect tag and vers_ts->tag from NULL
+       dereference.
+       [Fixes NetBSD coverity cid-1053.]
+
 2006-05-04  Mark D. Baushke  <address@hidden>
 
-       * filesubr.c (cvs_temp_file): Avoid keeping pointers to free()'d
-       storage laying around.
-       * commit.c (commit): Handle possible NULL filename values
-       returned from cvs_temp_file().
-       * filesubr.c (cvs_temp_name): Ditto.
-       * import.c (import): Ditto.
-       * login.c (password_entry_operation): Ditto.
-       * logmsg.c (do_verify): Ditto.
-       * patch.c (patch_fileproc): Ditto.
-       [Fixes NetBSD coverity cid-2545.]
+       * filesubr.c (cvs_temp_file): Avoid keeping pointers to free()'d
+       storage laying around.
+       * commit.c (commit): Handle possible NULL filename values
+       returned from cvs_temp_file().
+       * filesubr.c (cvs_temp_name): Ditto.
+       * import.c (import): Ditto.
+       * login.c (password_entry_operation): Ditto.
+       * logmsg.c (do_verify): Ditto.
+       * patch.c (patch_fileproc): Ditto.
+       [Fixes NetBSD coverity cid-2545.]
 
-       * buffer.c (packetizing_buffer_output): Initialize outdata.
-       [Fixes NetBSD coverity cid-2474.]
+       * buffer.c (packetizing_buffer_output): Initialize outdata.
+       [Fixes NetBSD coverity cid-2474.]
 
        * server.c (server_updated): Fix NetBSD coverity cid-1352
        NetBSD-sparc64 of 2006-May-02 03:02:46.
Index: log.c
===================================================================
RCS file: /cvsroot/src/gnu/dist/xcvs/src/log.c,v
retrieving revision 1.3
diff -u -p -r1.3 log.c
--- log.c       12 May 2006 15:33:17 -0000      1.3
+++ log.c       12 May 2006 21:38:24 -0000
@@ -1092,6 +1092,8 @@ log_expand_revlist (rcs, baserev, revlis
        {
            char *branch;
 
+           assert (r->first != NULL);
+
            /* Print just the head of the branch.  */
            if (isdigit ((unsigned char) r->first[0]))
                nr->first = RCS_getbranch (rcs, r->first, 1);
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    12 May 2006 21:38:24 -0000
@@ -3041,6 +3041,7 @@ error  \n");
        {
            buf_output0 (buf_to_net, "E close failed\n");
            print_error (errno);
+           dev_null_fd = -1;   /* Do not try to close it again. */
            goto error_exit;
        }
        dev_null_fd = -1;
@@ -3378,7 +3379,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]);