Clean up <rev> handling. Whenever a revision is specified after a

flag, it calls one of two new functions: rcs_setrevstr() or
rcs_setrevstr2().  rcs_setrevstr() sets a string to another string,
and complains if it was set more than once.  rcs_setrevstr2() takes
two strings, sets one after the other, and fatal()s if more than
two strings were given.

All <rev> handling is now done in the loop that goes through each
argv.  This is necessary for parsing symbols, which will be much
easier after this.

Along the way a lot of memory leaks were cleaned up.  There is one
area where rcs_set_rev() is called, which allocates a RCSNUM and
stores it in pb.newrev, but it segfaults whenever I try to rcsnum_free()
it.  I put an /* XXX */ comment there for now.

Passes regression tests and the code is less complicated in some
ways (to me).

Suggestions and OK xsa@

7 files changed, 154 insertions, 95 deletions

diff --git a/usr.bin/rcs/ci.c b/usr.bin/rcs/ci.c
index ec7ccaa1ee9..36fc4e345dd 100644
--- a/usr.bin/rcs/ci.c
+++ b/usr.bin/rcs/ci.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ci.c,v 1.138 2006/04/10 19:49:44 joris Exp $	*/
+/*	$OpenBSD: ci.c,v 1.139 2006/04/12 08:23:30 ray Exp $	*/
 /*
  * Copyright (c) 2005, 2006 Niall O'Higgins <niallo@openbsd.org>
  * All rights reserved.
@@ -92,8 +92,6 @@ checkin_usage(void)
 	    "	  [-u[rev]] [-wusername] [-xsuffixes] [-ztz] file ...\n");
 }
 
-
-
 /*
  * checkin_main()
  *
@@ -104,6 +102,7 @@ int
 checkin_main(int argc, char **argv)
 {
 	int i, ch, status;
+	char *rev_str;
 	struct checkin_params pb;
 
 	pb.date = DATE_NOW;
@@ -113,9 +112,9 @@ checkin_main(int argc, char **argv)
 	pb.newrev =  NULL;
 	pb.flags = status = 0;
 	pb.fmode = S_IRUSR|S_IRGRP|S_IROTH;
-
 	pb.flags = INTERACTIVE;
 	pb.openflags = RCS_RDWR|RCS_CREATE|RCS_PARSE_FULLY;
+	rev_str = NULL;
 
 	while ((ch = rcs_getopt(argc, argv, CI_OPTSTRING)) != -1) {
 		switch (ch) {
@@ -126,7 +125,7 @@ checkin_main(int argc, char **argv)
 				fatal("invalid date");
 			break;
 		case 'f':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.flags |= FORCE;
 			break;
 		case 'h':
@@ -134,29 +133,29 @@ checkin_main(int argc, char **argv)
 			exit(0);
 			/* NOTREACHED */
 		case 'I':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.flags |= INTERACTIVE;
 			break;
 		case 'i':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.openflags |= RCS_CREATE;
 			pb.flags |= CI_INIT;
 			break;
 		case 'j':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.openflags &= ~RCS_CREATE;
 			pb.flags &= ~CI_INIT;
 			break;
 		case 'k':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.flags |= CI_KEYWORDSCAN;
 			break;
 		case 'l':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.flags |= CO_LOCK;
 			break;
 		case 'M':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.flags |= CO_REVDATE;
 			break;
 		case 'm':
@@ -180,7 +179,7 @@ checkin_main(int argc, char **argv)
 			verbose = 0;
 			break;
 		case 'r':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.flags |= CI_DEFAULT;
 			break;
 		case 's':
@@ -195,7 +194,7 @@ checkin_main(int argc, char **argv)
 			pb.description = xstrdup(rcs_optarg);
 			break;
 		case 'u':
-			rcs_set_rev(rcs_optarg, &pb.newrev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pb.flags |= CO_UNLOCK;
 			break;
 		case 'V':
@@ -276,6 +275,10 @@ checkin_main(int argc, char **argv)
 		if (verbose == 1)
 			printf("%s  <--  %s\n", pb.fpath, pb.filename);
 
+		/* XXX - Should we rcsnum_free(pb.newrev)? */
+		if (rev_str != NULL)
+			rcs_set_rev(rev_str, &pb.newrev);
+
 		if (pb.flags & NEWFILE)
 			status = checkin_init(&pb);
 		else
diff --git a/usr.bin/rcs/co.c b/usr.bin/rcs/co.c
index 0a335b1f2bf..35fa43990a9 100644
--- a/usr.bin/rcs/co.c
+++ b/usr.bin/rcs/co.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: co.c,v 1.72 2006/04/10 08:08:00 xsa Exp $	*/
+/*	$OpenBSD: co.c,v 1.73 2006/04/12 08:23:30 ray Exp $	*/
 /*
  * Copyright (c) 2005 Joris Vink <joris@openbsd.org>
  * All rights reserved.
@@ -40,7 +40,7 @@ checkout_main(int argc, char **argv)
 	RCSNUM *frev, *rev;
 	RCSFILE *file;
 	char fpath[MAXPATHLEN];
-	char *author, *username, *date;
+	char *author, *date, *rev_str, *username;
 	const char *state;
 	time_t rcs_mtime = -1;
 
@@ -48,6 +48,7 @@ checkout_main(int argc, char **argv)
 	kflag = RCS_KWEXP_ERR;
 	rev = RCS_HEAD_REV;
 	frev = NULL;
+	rev_str = NULL;
 	state = NULL;
 	author = NULL;
 	date = NULL;
@@ -58,11 +59,11 @@ checkout_main(int argc, char **argv)
 			date = xstrdup(rcs_optarg);
 			break;
 		case 'f':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			flags |= FORCE;
 			break;
 		case 'I':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			flags |= INTERACTIVE;
 			break;
 
@@ -76,27 +77,27 @@ checkout_main(int argc, char **argv)
 			}
 			break;
 		case 'l':
-			rcs_set_rev(rcs_optarg, &rev);
 			if (flags & CO_UNLOCK) {
 				cvs_log(LP_ERR, "warning: -u overridden by -l");
 				flags &= ~CO_UNLOCK;
 			}
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			flags |= CO_LOCK;
 			break;
 		case 'M':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			flags |= CO_REVDATE;
 			break;
 		case 'p':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			pipeout = 1;
 			break;
 		case 'q':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			verbose = 0;
 			break;
 		case 'r':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			break;
 		case 's':
 			state = xstrdup(rcs_optarg);
@@ -106,7 +107,7 @@ checkout_main(int argc, char **argv)
 			flags |= PRESERVETIME;
 			break;
 		case 'u':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			if (flags & CO_LOCK) {
 				cvs_log(LP_ERR, "warning: -l overridden by -u");
 				flags &= ~CO_LOCK;
@@ -175,21 +176,25 @@ checkout_main(int argc, char **argv)
 
 		rcs_kwexp_set(file, kflag);
 
-		if (rev == RCS_HEAD_REV)
-			frev = file->rf_head;
-		else
-			frev = rev;
+		if (rev_str != NULL)
+			rcs_set_rev(rev_str, &rev);
+		else {
+			rev = rcsnum_alloc();
+			rcsnum_cpy(file->rf_head, rev, 0);
+		}
 
-		if ((status = checkout_rev(file, frev, argv[i], flags,
+		if ((status = checkout_rev(file, rev, argv[i], flags,
 		    username, author, state, date)) < 0) {
-				rcs_close(file);
-				continue;
+			rcs_close(file);
+			rcsnum_free(rev);
+			continue;
 		}
 
 		if (verbose == 1)
 			printf("done\n");
 
 		rcs_close(file);
+		rcsnum_free(rev);
 
 		if (flags & PRESERVETIME)
 			rcs_set_mtime(fpath, rcs_mtime);
diff --git a/usr.bin/rcs/rcsclean.c b/usr.bin/rcs/rcsclean.c
index bd17fc11522..83a76f96f3c 100644
--- a/usr.bin/rcs/rcsclean.c
+++ b/usr.bin/rcs/rcsclean.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rcsclean.c,v 1.29 2006/04/10 08:08:00 xsa Exp $	*/
+/*	$OpenBSD: rcsclean.c,v 1.30 2006/04/12 08:23:30 ray Exp $	*/
 /*
  * Copyright (c) 2005 Joris Vink <joris@openbsd.org>
  * All rights reserved.
@@ -41,11 +41,13 @@ int
 rcsclean_main(int argc, char **argv)
 {
 	int i, ch;
+	char *rev_str;
 	RCSNUM *rev;
 	DIR *dirp;
 	struct dirent *dp;
 
 	rev = RCS_HEAD_REV;
+	rev_str = NULL;
 
 	while ((ch = rcs_getopt(argc, argv, "k:n::q::r:Tu::Vx::")) != -1) {
 		switch (ch) {
@@ -59,21 +61,21 @@ rcsclean_main(int argc, char **argv)
 			}
 			break;
 		case 'n':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			nflag = 1;
 			break;
 		case 'q':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			verbose = 0;
 			break;
 		case 'r':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			break;
 		case 'T':
 			flags |= PRESERVETIME;
 			break;
 		case 'u':
-			rcs_set_rev(rcs_optarg, &rev);
+			rcs_setrevstr(&rev_str, rcs_optarg);
 			uflag = 1;
 			break;
 		case 'V':
@@ -105,14 +107,18 @@ rcsclean_main(int argc, char **argv)
 		while ((dp = readdir(dirp)) != NULL) {
 			if (dp->d_type == DT_DIR)
 				continue;
+			rcs_set_rev(rev_str, &rev);
 			rcsclean_file(dp->d_name, rev);
+			rcsnum_free(rev);
 		}
 
 		closedir(dirp);
-	} else {
-		for (i = 0; i < argc; i++)
+	} else
+		for (i = 0; i < argc; i++) {
+			rcs_set_rev(rev_str, &rev);
 			rcsclean_file(argv[i], rev);
-	}
+			rcsnum_free(rev);
+		}
 
 	return (0);
 }
diff --git a/usr.bin/rcs/rcsdiff.c b/usr.bin/rcs/rcsdiff.c
index 8178280ee2c..fa9b3f0f8b3 100644
--- a/usr.bin/rcs/rcsdiff.c
+++ b/usr.bin/rcs/rcsdiff.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rcsdiff.c,v 1.45 2006/04/10 19:49:45 joris Exp $	*/
+/*	$OpenBSD: rcsdiff.c,v 1.46 2006/04/12 08:23:30 ray Exp $	*/
 /*
  * Copyright (c) 2005 Joris Vink <joris@openbsd.org>
  * All rights reserved.
@@ -38,12 +38,12 @@ int
 rcsdiff_main(int argc, char **argv)
 {
 	int i, ch, status;
-	RCSNUM *rev, *rev2, *frev;
+	RCSNUM *rev1, *rev2;
 	RCSFILE *file;
-	char fpath[MAXPATHLEN];
+	char fpath[MAXPATHLEN], *rev_str1, *rev_str2;
 
-	rev = RCS_HEAD_REV;
-	rev2 = NULL;
+	rev1 = rev2 = NULL;
+	rev_str1 = rev_str2 = NULL;
 	status = 0;
 
 	strlcpy(diffargs, "diff", sizeof(diffargs));
@@ -75,13 +75,7 @@ rcsdiff_main(int argc, char **argv)
 			diff_format = D_UNIFIED;
 			break;
 		case 'r':
-			if (rev == RCS_HEAD_REV) {
-				if ((rev = rcsnum_parse(rcs_optarg)) == NULL)
-					fatal("bad revision number");
-			} else {
-				if ((rev2 = rcsnum_parse(rcs_optarg)) == NULL)
-					fatal("bad revision number");
-			}
+			rcs_setrevstr2(&rev_str1, &rev_str2, rcs_optarg);
 			break;
 		case 'T':
 			/*
@@ -123,10 +117,14 @@ rcsdiff_main(int argc, char **argv)
 
 		rcs_kwexp_set(file, kflag);
 
-		if (rev == RCS_HEAD_REV)
-			frev = file->rf_head;
-		else
-			frev = rev;
+		if (rev_str1 != NULL) {
+			if ((rev1 = rcsnum_parse(rev_str1)) == NULL)
+				fatal("bad revision number");
+		}
+		if (rev_str2 != NULL) {
+			if ((rev2 = rcsnum_parse(rev_str2)) == NULL)
+				fatal("bad revision number");
+		}
 
 		if (verbose == 1) {
 			fprintf(stderr, "%s\n", RCS_DIFF_DIV);
@@ -135,21 +133,30 @@ rcsdiff_main(int argc, char **argv)
 
 		diff_file = argv[i];
 
-		if (rev2 == NULL) {
-			if (rcsdiff_file(file, frev, argv[i]) < 0) {
-				rcs_close(file);
+		/* No revisions given. */
+		if (rev_str1 == NULL) {
+			if (rcsdiff_file(file, file->rf_head, argv[i]) < 0)
 				status = 2;
-				continue;
-			}
+		/* One revision given. */
+		} else if (rev_str2 == NULL) {
+			if (rcsdiff_file(file, rev1, argv[i]) < 0)
+				status = 2;
+		/* Two revisions given. */
 		} else {
-			if (rcsdiff_rev(file, rev, rev2) < 0) {
-				rcs_close(file);
+			if (rcsdiff_rev(file, rev1, rev2) < 0)
 				status = 2;
-				continue;
-			}
 		}
 
 		rcs_close(file);
+
+		if (rev1 != NULL) {
+			rcsnum_free(rev1);
+			rev1 = NULL;
+		}
+		if (rev2 != NULL) {
+			rcsnum_free(rev2);
+			rev2 = NULL;
+		}
 	}
 
 	return (status);
diff --git a/usr.bin/rcs/rcsmerge.c b/usr.bin/rcs/rcsmerge.c
index 61679e4998b..4d591d11bd2 100644
--- a/usr.bin/rcs/rcsmerge.c
+++ b/usr.bin/rcs/rcsmerge.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rcsmerge.c,v 1.23 2006/03/24 05:14:48 ray Exp $	*/
+/*	$OpenBSD: rcsmerge.c,v 1.24 2006/04/12 08:23:30 ray Exp $	*/
 /*
  * Copyright (c) 2005, 2006 Xavier Santolaria <xsa@openbsd.org>
  * All rights reserved.
@@ -35,12 +35,13 @@ int
 rcsmerge_main(int argc, char **argv)
 {
 	int i, ch;
-	char *fcont, fpath[MAXPATHLEN], r1[16], r2[16];
+	char *fcont, fpath[MAXPATHLEN], r1[16], r2[16], *rev_str1, *rev_str2;
 	RCSFILE *file;
-	RCSNUM *baserev, *rev2, *frev;
+	RCSNUM *rev1, *rev2;
 	BUF *bp;
 
-	baserev = rev2 = RCS_HEAD_REV;
+	rev1 = rev2 = NULL;
+	rev_str1 = rev_str2 = NULL;
 
 	while ((ch = rcs_getopt(argc, argv, "AEek:p::q::r:TVx::z:")) != -1) {
 		switch (ch) {
@@ -56,20 +57,15 @@ rcsmerge_main(int argc, char **argv)
 			}
 			break;
 		case 'p':
-			rcs_set_rev(rcs_optarg, &baserev);
+			rcs_setrevstr2(&rev_str1, &rev_str2, rcs_optarg);
 			pipeout = 1;
 			break;
 		case 'q':
-			rcs_set_rev(rcs_optarg, &baserev);
+			rcs_setrevstr2(&rev_str1, &rev_str2, rcs_optarg);
 			verbose = 0;
 			break;
 		case 'r':
-			if (baserev == RCS_HEAD_REV)
-				rcs_set_rev(rcs_optarg, &baserev);
-			else if (rev2 == RCS_HEAD_REV)
-				rcs_set_rev(rcs_optarg, &rev2);
-			else
-				fatal("too many revision numbers");
+			rcs_setrevstr2(&rev_str1, &rev_str2, rcs_optarg);
 			break;
 		case 'T':
 			/*
@@ -101,7 +97,7 @@ rcsmerge_main(int argc, char **argv)
 		exit(1);
 	}
 
-	if (baserev == RCS_HEAD_REV) {
+	if (rev_str1 == NULL) {
 		cvs_log(LP_ERR, "no base revision number given");
 		(usage)();
 		exit(1);
@@ -117,26 +113,39 @@ rcsmerge_main(int argc, char **argv)
 		if (verbose == 1)
 			fprintf(stderr, "RCS file: %s\n", fpath);
 
-		if (rev2 == RCS_HEAD_REV)
-			frev = file->rf_head;
-		else
-			frev = rev2;
+		if (rev1 != NULL) {
+			rcsnum_free(rev1);
+			rev1 = NULL;
+		}
+		if (rev2 != NULL) {
+			rcsnum_free(rev2);
+			rev2 = NULL;
+		}
+
+		rcs_set_rev(rev_str1, &rev1);
+		if (rev_str2 != NULL)
+			rcs_set_rev(rev_str2, &rev2);
+		else {
+			rev2 = rcsnum_alloc();
+			rcsnum_cpy(file->rf_head, rev2, 0);
+		}
 
-		if (rcsnum_cmp(baserev, frev, 0) == 0) {
+		if (rcsnum_cmp(rev1, rev2, 0) == 0) {
 			rcs_close(file);
 			continue;
 		}
 
-		rcsnum_tostr(baserev, r1, sizeof(r1));
-		rcsnum_tostr(frev, r2, sizeof(r2));
+		if (verbose == 1) {
+			(void)rcsnum_tostr(rev1, r1, sizeof(r1));
+			(void)rcsnum_tostr(rev2, r2, sizeof(r2));
 
-		if (verbose == 1)
 			fprintf(stderr, "Merging differences between %s and "
 			    "%s into %s%s\n", r1, r2, argv[i],
 			    (pipeout == 1) ? "; result to stdout":"");
+		}
 
-		if ((bp = cvs_diff3(file, argv[i], baserev,
-		    frev, verbose)) == NULL) {
+		if ((bp = cvs_diff3(file, argv[i], rev1, rev2,
+		    verbose)) == NULL) {
 			cvs_log(LP_ERR, "failed to merge");
 			rcs_close(file);
 			continue;
diff --git a/usr.bin/rcs/rcsprog.c b/usr.bin/rcs/rcsprog.c
index d69d6e9a433..33045e38742 100644
--- a/usr.bin/rcs/rcsprog.c
+++ b/usr.bin/rcs/rcsprog.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rcsprog.c,v 1.96 2006/04/11 08:07:35 ray Exp $	*/
+/*	$OpenBSD: rcsprog.c,v 1.97 2006/04/12 08:23:30 ray Exp $	*/
 /*
  * Copyright (c) 2005 Jean-Francois Brousseau <jfb@openbsd.org>
  * All rights reserved.
@@ -75,16 +75,13 @@ sighdlr(int sig)
 	_exit(1);
 }
 
+/*
+ * Allocate an RCSNUM and store in <rev>.
+ */
 void
 rcs_set_rev(const char *str, RCSNUM **rev)
 {
-	if (str == NULL)
-		return;
-
-	if ((*rev != NULL) && (*rev != RCS_HEAD_REV))
-		cvs_log(LP_WARN, "redefinition of revision number");
-
-	if ((*rev = rcsnum_parse(str)) == NULL)
+	if (str == NULL || (*rev = rcsnum_parse(str)) == NULL)
 		fatal("bad revision number '%s'", str);
 }
 
@@ -361,6 +358,36 @@ rcs_statfile(char *fname, char *out, size_t len)
 	return (0);
 }
 
+/*
+ * Set <str> to <new_str>.  Print warning if <str> is redefined.
+ */
+void
+rcs_setrevstr(char **str, char *new_str)
+{
+	if (new_str == NULL)
+		return;
+	if (*str != NULL)
+		cvs_log(LP_WARN, "redefinition of revision number");
+	*str = new_str;
+}
+
+/*
+ * Set <str1> or <str2> to <new_str>, depending on which is not set.
+ * If both are set, error out.
+ */
+void
+rcs_setrevstr2(char **str1, char **str2, char *new_str)
+{
+	if (new_str == NULL)
+		return;
+	if (*str1 == NULL)
+		*str1 = new_str;
+	else if (*str2 == NULL)
+		*str2 = new_str;
+	else
+		fatal("too many revision numbers");
+}
+
 int
 main(int argc, char **argv)
 {
diff --git a/usr.bin/rcs/rcsprog.h b/usr.bin/rcs/rcsprog.h
index 2e99a90f8e0..1810c6bf085 100644
--- a/usr.bin/rcs/rcsprog.h
+++ b/usr.bin/rcs/rcsprog.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rcsprog.h,v 1.43 2006/04/09 19:22:23 niallo Exp $	*/
+/*	$OpenBSD: rcsprog.h,v 1.44 2006/04/12 08:23:30 ray Exp $	*/
 /*
  * Copyright (c) 2005 Joris Vink <joris@openbsd.org>
  * All rights reserved.
@@ -113,6 +113,8 @@ char 	*rcs_choosefile(const char *);
 int	rcs_statfile(char *, char *, size_t);
 time_t	rcs_get_mtime(const char *);
 void	rcs_set_rev(const char *, RCSNUM **);
+void	rcs_setrevstr(char **, char *);
+void	rcs_setrevstr2(char **, char **, char *);
 void	rcs_usage(void);
 void	(*usage)(void);