From d25db4db5902d5bc6273e61c06b816dfaa1bd335 Mon Sep 17 00:00:00 2001 From: christos Date: Sat, 27 May 2006 03:01:09 +0000 Subject: [PATCH] Coverity fixes from Mark D. Baushke --- gnu/dist/xcvs/ChangeLog | 33 +++++++++++++++++ gnu/dist/xcvs/src/add.c | 4 +++ gnu/dist/xcvs/src/edit.c | 8 +++-- gnu/dist/xcvs/src/lock.c | 3 ++ gnu/dist/xcvs/src/rcs.c | 8 ++++- gnu/dist/xcvs/src/server.c | 73 ++++++++++++++++++++++++++------------ gnu/dist/xcvs/src/tag.c | 8 ++++- 7 files changed, 111 insertions(+), 26 deletions(-) diff --git a/gnu/dist/xcvs/ChangeLog b/gnu/dist/xcvs/ChangeLog index aeb42de266b4..2749d4aa27b4 100644 --- a/gnu/dist/xcvs/ChangeLog +++ b/gnu/dist/xcvs/ChangeLog @@ -1,3 +1,36 @@ +2006-05-25 Mark D. Baushke + + * 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.] + 2006-05-12 Mark D. Baushke * rcs.c (RCS_isdead): Assert that the first argument is not NULL. diff --git a/gnu/dist/xcvs/src/add.c b/gnu/dist/xcvs/src/add.c index ff205863f455..0e9fb59bfbed 100644 --- a/gnu/dist/xcvs/src/add.c +++ b/gnu/dist/xcvs/src/add.c @@ -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 (); diff --git a/gnu/dist/xcvs/src/edit.c b/gnu/dist/xcvs/src/edit.c index 471085de0276..579cc47d6a0d 100644 --- a/gnu/dist/xcvs/src/edit.c +++ b/gnu/dist/xcvs/src/edit.c @@ -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, repository, update_dir, entries) { 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; } diff --git a/gnu/dist/xcvs/src/lock.c b/gnu/dist/xcvs/src/lock.c index cb07372a5db6..389cac8daca6 100644 --- a/gnu/dist/xcvs/src/lock.c +++ b/gnu/dist/xcvs/src/lock.c @@ -718,6 +718,9 @@ readers_exist (repository) #endif lockdir = lock_name (repository, ""); + + assert (lockdir != NULL); + lockdir[strlen (lockdir) - 1] = '\0'; /* remove trailing slash */ do { diff --git a/gnu/dist/xcvs/src/rcs.c b/gnu/dist/xcvs/src/rcs.c index 0f93385238fc..e9f1af392281 100644 --- a/gnu/dist/xcvs/src/rcs.c +++ b/gnu/dist/xcvs/src/rcs.c @@ -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.", diff --git a/gnu/dist/xcvs/src/server.c b/gnu/dist/xcvs/src/server.c index 0e06a24e29cc..6b3269d7b5b4 100644 --- a/gnu/dist/xcvs/src/server.c +++ b/gnu/dist/xcvs/src/server.c @@ -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, repository) const char *update_dir; const char *repository; { + assert (file); + assert (update_dir); + assert (repository); + if (noexec) return; if (scratched_file != NULL && entries_line == NULL) diff --git a/gnu/dist/xcvs/src/tag.c b/gnu/dist/xcvs/src/tag.c index ef9e57473b31..fca367f0571c 100644 --- a/gnu/dist/xcvs/src/tag.c +++ b/gnu/dist/xcvs/src/tag.c @@ -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.