crontab: almost complete rewrite

crontab: make options consistent with other implementations

   text    data     bss     dec     hex filename
   2042       4       0    2046     7fe busybox.t1/miscutils/crontab.o
   1331       0       0    1331     533 busybox.t2/miscutils/crontab.o

function                                             old     new   delta
edit_file                                            733     956    +223
open_as_user                                           -     171    +171
packed_usage                                       23652   23650      -2
CDir                                                   8       4      -4
ChangeUser                                           139       -    -139
crontab_main                                        1522     616    -906
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 1/3 up/down: 394/-1051)        Total: -657 bytes

2 files changed, 194 insertions, 291 deletions

diff --git a/include/usage.h b/include/usage.h
index 71e1d125f..59e57f3a0 100644
--- a/include/usage.h
+++ b/include/usage.h
@@ -510,15 +510,14 @@ USE_FEATURE_BRCTL_FANCY("\n" \
        "	-c dir	Working dir"
 
 #define crontab_trivial_usage \
-       "[-c dir] {file|-}|[-u|-l|-e|-d user]"
+       "[-c DIR] [-u USER] [-ler]|[FILE]"
 #define crontab_full_usage \
-       "	file [opts]	Replace crontab from file\n" \
-       "	- [opts]	Replace crontab from stdin\n" \
-       "	-u user		User\n" \
-       "	-l [user]	List crontab for user\n" \
-       "	-e [user]	Edit crontab for user\n" \
-       "	-d [user]	Delete crontab for user\n" \
-       "	-c dir		Crontab directory"
+       "	-c	Crontab directory" \
+     "\n	-u	User" \
+     "\n	-l	List crontab" \
+     "\n	-e	Edit crontab" \
+     "\n	-r	Delete crontab" \
+     "\n	FILE	Replace crontab by FILE ('-': stdin)" \
 
 #define cryptpw_trivial_usage \
        "[-a des|md5] [string]"
diff --git a/miscutils/crontab.c b/miscutils/crontab.c
index 6d245da90..71037b7ee 100644
--- a/miscutils/crontab.c
+++ b/miscutils/crontab.c
@@ -25,321 +25,225 @@
 #define PATH_VI         "/bin/vi"   /* location of vi */
 #endif
 
-static const char *CDir = CRONTABS;
-
-static void EditFile(const char *user, const char *file);
-static int GetReplaceStream(const char *user, const char *file);
-static int ChangeUser(const char *user, short dochdir);
-
-int crontab_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
-int crontab_main(int ac, char **av)
+static void change_user(const struct passwd *pas)
 {
-	enum { NONE, EDIT, LIST, REPLACE, DELETE } option = NONE;
-	const struct passwd *pas;
-	const char *repFile = NULL;
-	int repFd = 0;
-	int i;
-	char caller[256];           /* user that ran program */
-	char buf[1024];
-	int UserId;
-
-	UserId = getuid();
-	pas = getpwuid(UserId);
-	if (pas == NULL)
-		bb_perror_msg_and_die("getpwuid");
-
-	safe_strncpy(caller, pas->pw_name, sizeof(caller));
-
-	i = 1;
-	if (ac > 1) {
-		if (LONE_DASH(av[1])) {
-			option = REPLACE;
-			++i;
-		} else if (av[1][0] != '-') {
-			option = REPLACE;
-			++i;
-			repFile = av[1];
-		}
-	}
+	setenv("USER", pas->pw_name, 1);
+	setenv("HOME", pas->pw_dir, 1);
+	setenv("SHELL", DEFAULT_SHELL, 1);
 
-	for (; i < ac; ++i) {
-		char *ptr = av[i];
+	/* initgroups, setgid, setuid */
+	change_identity(pas);
 
-		if (*ptr != '-')
-			break;
-		ptr += 2;
-
-		switch (ptr[-1]) {
-		case 'l':
-			if (ptr[-1] == 'l')
-				option = LIST;
-			/* fall through */
-		case 'e':
-			if (ptr[-1] == 'e')
-				option = EDIT;
-			/* fall through */
-		case 'd':
-			if (ptr[-1] == 'd')
-				option = DELETE;
-			/* fall through */
-		case 'u':
-			if (i + 1 < ac && av[i+1][0] != '-') {
-				++i;
-				if (getuid() == geteuid()) {
-					pas = getpwnam(av[i]);
-					if (pas) {
-						UserId = pas->pw_uid;
-					} else {
-						bb_error_msg_and_die("user %s unknown", av[i]);
-					}
-				} else {
-					bb_error_msg_and_die("only the superuser may specify a user");
-				}
-			}
-			break;
-		case 'c':
-			if (getuid() == geteuid()) {
-				CDir = (*ptr) ? ptr : av[++i];
-			} else {
-				bb_error_msg_and_die("-c option: superuser only");
-			}
-			break;
-		default:
-			i = ac;
-			break;
-		}
+	if (chdir(pas->pw_dir) < 0) {
+		bb_perror_msg("chdir(%s) by %s failed",
+				pas->pw_dir, pas->pw_name);
+		xchdir(TMPDIR);
 	}
-	if (i != ac || option == NONE)
-		bb_show_usage();
-
-	/*
-	 * Get password entry
-	 */
-
-	pas = getpwuid(UserId);
-	if (pas == NULL)
-		bb_perror_msg_and_die("getpwuid");
-
-	/*
-	 * If there is a replacement file, obtain a secure descriptor to it.
-	 */
+}
 
-	if (repFile) {
-		repFd = GetReplaceStream(caller, repFile);
-		if (repFd < 0)
-			bb_error_msg_and_die("cannot read replacement file");
+static void edit_file(const struct passwd *pas, const char *file)
+{
+	const char *ptr;
+	int pid = vfork();
+
+	if (pid < 0) /* failure */
+		bb_perror_msg_and_die("vfork");
+	if (pid) { /* parent */
+		wait4pid(pid);
+		return;
 	}
 
-	/*
-	 * Change directory to our crontab directory
-	 */
-
-	xchdir(CDir);
+	/* CHILD - change user and run editor */
+	change_user(pas);
+	ptr = getenv("VISUAL");
+	if (!ptr) {
+		ptr = getenv("EDITOR");
+		if (!ptr)
+			ptr = PATH_VI;
+	}
 
-	/*
-	 * Handle options as appropriate
-	 */
+	/* TODO: clean up the environment!!! */
+	/* not execlp - we won't use PATH */
+	execl(ptr, ptr, file, NULL);
+	bb_perror_msg_and_die("exec %s", ptr);
+}
 
-	switch (option) {
-	case LIST:
-		{
-			FILE *fi;
-
-			fi = fopen(pas->pw_name, "r");
-			if (fi) {
-				while (fgets(buf, sizeof(buf), fi) != NULL)
-					fputs(buf, stdout);
-				fclose(fi);
-			} else {
-				bb_error_msg("no crontab for %s", pas->pw_name);
-			}
-		}
-		break;
-	case EDIT:
-		{
-/* FIXME: messy code here! we have file copying helpers for this! */
-			FILE *fi;
-			int fd;
-			int n;
-			char tmp[128];
-
-			snprintf(tmp, sizeof(tmp), TMPDIR "/crontab.%d", getpid());
-			fd = xopen3(tmp, O_RDWR|O_CREAT|O_TRUNC|O_EXCL, 0600);
-/* race, use fchown */
-			chown(tmp, getuid(), getgid());
-			fi = fopen(pas->pw_name, "r");
-			if (fi) {
-				while ((n = fread(buf, 1, sizeof(buf), fi)) > 0)
-					full_write(fd, buf, n);
-			}
-			EditFile(caller, tmp);
-			remove(tmp);
-			lseek(fd, 0L, SEEK_SET);
-			repFd = fd;
-		}
-		option = REPLACE;
-		/* fall through */
-	case REPLACE:
-		{
-/* same here */
-			char path[1024];
-			int fd;
-			int n;
-
-			snprintf(path, sizeof(path), "%s.new", pas->pw_name);
-			fd = open(path, O_CREAT|O_TRUNC|O_APPEND|O_WRONLY, 0600);
-			if (fd >= 0) {
-				while ((n = read(repFd, buf, sizeof(buf))) > 0) {
-					full_write(fd, buf, n);
-				}
-				close(fd);
-				rename(path, pas->pw_name);
-			} else {
-				bb_error_msg("cannot create %s/%s", CDir, path);
-			}
-			close(repFd);
-		}
-		break;
-	case DELETE:
-		remove(pas->pw_name);
-		break;
-	case NONE:
-	default:
-		break;
+static int open_as_user(const struct passwd *pas, const char *file)
+{
+	int filedes[2];
+	pid_t pid;
+	char c;
+
+	xpipe(filedes);
+	pid = vfork();
+	if (pid < 0) /* ERROR */
+		bb_perror_msg_and_die("vfork");
+	if (pid) { /* PARENT */
+		int n = safe_read(filedes[0], &c, 1);
+		close(filedes[0]);
+		close(filedes[1]);
+		if (n > 0) /* child says it can read */
+			return open(file, O_RDONLY);
+		return -1;
 	}
 
-	/*
-	 *  Bump notification file.  Handle window where crond picks file up
-	 *  before we can write our entry out.
-	 */
+	/* CHILD */
 
-	if (option == REPLACE || option == DELETE) {
-		FILE *fo;
-		struct stat st;
+	/* initgroups, setgid, setuid */
+	change_identity(pas);
 
-		while ((fo = fopen(CRONUPDATE, "a"))) {
-			fprintf(fo, "%s\n", pas->pw_name);
-			fflush(fo);
-			if (fstat(fileno(fo), &st) != 0 || st.st_nlink != 0) {
-				fclose(fo);
-				break;
-			}
-			fclose(fo);
-			/* loop */
-		}
-		if (fo == NULL) {
-			bb_error_msg("cannot append to %s/%s", CDir, CRONUPDATE);
-		}
-	}
-	return 0;
+	/* We just try to read one byte. If that works, file is readable
+	 * under this user. We signal that by sending one byte to parent. */
+	if (safe_read(xopen(file, O_RDONLY), &c, 1) == 1)
+		safe_write(filedes[1], &c, 1); /* "papa, I can read!" */
+	_exit(0);
 }
 
-static int GetReplaceStream(const char *user, const char *file)
+int crontab_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
+int crontab_main(int argc, char **argv)
 {
-	int filedes[2];
-	int pid;
+	const struct passwd *pas;
+	const char *crontab_dir = CRONTABS;
+	char *tmp_fname;
+	char *new_fname;
+	char *user_name;  /* -u USER */
 	int fd;
-	int n;
-	char buf[1024];
-
-	if (pipe(filedes) < 0) {
-		perror("pipe");
-		return -1;
-	}
-	pid = fork();
-	if (pid < 0) {
-		perror("fork");
-		return -1;
+	int opt_ler;
+	uid_t my_uid;
+
+	/* file [opts]     Replace crontab from file
+	 * - [opts]        Replace crontab from stdin
+	 * -u user         User
+	 * -c dir          Crontab directory
+	 * -l              List crontab for user
+	 * -e              Edit crontab for user
+	 * -r              Delete crontab for user
+	 * bbox also supports -d == -r, but most other crontab
+	 * implementations do not. Deprecated.
+	 */
+	enum {
+		OPT_u = (1 << 0),
+		OPT_c = (1 << 1),
+		OPT_l = (1 << 2),
+		OPT_e = (1 << 3),
+		OPT_r = (1 << 4),
+		OPT_ler = OPT_l + OPT_e + OPT_r,
+	};
+
+	my_uid = getuid();
+
+	opt_complementary = "?1:dr"; /* max one argument; -d implies -r */
+	opt_ler = getopt32(argv, "u:c:lerd", &user_name, &crontab_dir);
+	argv += optind;
+
+	if (opt_ler & (OPT_u|OPT_c))
+		if (my_uid != geteuid())
+			bb_error_msg_and_die("only root can use -c or -u");
+
+	if (opt_ler & OPT_u) {
+		pas = getpwnam(user_name);
+		if (!pas)
+			bb_error_msg_and_die("user %s is not known", user_name);
+		my_uid = pas->pw_uid;
+	} else {
+		pas = getpwuid(my_uid);
+		if (!pas)
+			bb_perror_msg_and_die("no user record for UID %u",
+					(unsigned)my_uid);
 	}
-	if (pid > 0) {
-		/*
-		 * PARENT
-		 */
 
-		close(filedes[1]);
-		if (read(filedes[0], buf, 1) != 1) {
-			close(filedes[0]);
-			filedes[0] = -1;
+#define user_name DONT_USE_ME_BEYOND_THIS_POINT
+#define my_uid    DONT_USE_ME_BEYOND_THIS_POINT
+
+	/* From now on, keep only -l, -e, -r bits */
+	opt_ler &= OPT_ler;
+	if ((opt_ler - 1) & opt_ler) /* more than one bit set? */
+		bb_show_usage();
+
+	/* Read replacement file under user's UID/GID/group vector */
+	if (!opt_ler) { /* Replace? */
+		if (!argv[0])
+			bb_show_usage();
+		if (NOT_LONE_DASH(argv[0])) {
+			fd = open_as_user(pas, argv[0]);
+			if (fd < 0)
+				bb_error_msg_and_die("user %s cannot read %s",
+						pas->pw_name, argv[0]);
+			xmove_fd(fd, STDIN_FILENO);
 		}
-		return filedes[0];
 	}
 
-	/*
-	 * CHILD
-	 */
+	/* cd to our crontab directory */
+	xchdir(crontab_dir);
 
-	close(filedes[0]);
+	tmp_fname = NULL;
 
-	if (ChangeUser(user, 0) < 0)
-		exit(0);
+	/* Handle requested operation */
+	switch (opt_ler) {
 
-	xfunc_error_retval = 0;
-	fd = xopen(file, O_RDONLY);
-	buf[0] = 0;
-	write(filedes[1], buf, 1);
-	while ((n = read(fd, buf, sizeof(buf))) > 0) {
-		write(filedes[1], buf, n);
-	}
-	exit(0);
-}
-
-static void EditFile(const char *user, const char *file)
-{
-	int pid = fork();
-
-	if (pid == 0) {
-		/*
-		 * CHILD - change user and run editor
-		 */
-		const char *ptr;
-
-		if (ChangeUser(user, 1) < 0)
-			exit(0);
-		ptr = getenv("VISUAL");
-		if (ptr == NULL)
-			ptr = getenv("EDITOR");
-		if (ptr == NULL)
-			ptr = PATH_VI;
+	default: /* case OPT_r: Delete */
+		remove(pas->pw_name);
+		break;
 
-		ptr = xasprintf("%s %s", ptr, file);
-		execl(DEFAULT_SHELL, DEFAULT_SHELL, "-c", ptr, NULL);
-		bb_perror_msg_and_die("exec");
-	}
-	if (pid < 0) {
-		/*
-		 * PARENT - failure
-		 */
-		bb_perror_msg_and_die("fork");
-	}
-	wait4(pid, NULL, 0, NULL);
-}
+	case OPT_l: /* List */
+		{
+			char *args[2] = { pas->pw_name, NULL };
+			return bb_cat(args);
+			/* list exits,
+			 * the rest go play with cron update file */
+		}
 
-static int ChangeUser(const char *user, short dochdir)
-{
-	struct passwd *pas;
+	case OPT_e: /* Edit */
+		tmp_fname = xasprintf(TMPDIR "/crontab.%u", (unsigned)getpid());
+		fd = xopen3(tmp_fname, O_RDWR|O_CREAT|O_TRUNC|O_EXCL, 0600);
+		xmove_fd(fd, STDIN_FILENO);
+		fd = open(pas->pw_name, O_RDONLY);
+		if (fd >= 0) {
+			bb_copyfd_eof(fd, STDIN_FILENO);
+			close(fd);
+		}
+		fchown(STDIN_FILENO, pas->pw_uid, pas->pw_gid);
+		edit_file(pas, tmp_fname);
+		xlseek(STDIN_FILENO, 0, SEEK_SET);
+		/* fall through */
 
-	/*
-	 * Obtain password entry and change privileges
-	 */
+	case 0: /* Replace (no -l, -e, or -r were given) */
+		new_fname = xasprintf("%s.new", pas->pw_name);
+		fd = open(new_fname, O_WRONLY|O_CREAT|O_TRUNC|O_APPEND, 0600);
+		if (fd >= 0) {
+			bb_copyfd_eof(STDIN_FILENO, fd);
+			close(fd);
+			rename(new_fname, pas->pw_name);
+		} else {
+			bb_error_msg("cannot create %s/%s",
+					crontab_dir, new_fname);
+		}
+		if (tmp_fname)
+			remove(tmp_fname);
+		/*free(tmp_fname);*/
+		/*free(new_fname);*/
 
-	pas = getpwnam(user);
-	if (pas == NULL) {
-		bb_perror_msg_and_die("cannot get uid for %s", user);
-	}
-	setenv("USER", pas->pw_name, 1);
-	setenv("HOME", pas->pw_dir, 1);
-	setenv("SHELL", DEFAULT_SHELL, 1);
+	} /* switch */
 
-	/*
-	 * Change running state to the user in question
+	/* Bump notification file.  Handle window where crond picks file up
+	 * before we can write our entry out.
 	 */
-	change_identity(pas);
+	while ((fd = open(CRONUPDATE, O_WRONLY|O_CREAT|O_APPEND)) >= 0) {
+		struct stat st;
 
-	if (dochdir) {
-		if (chdir(pas->pw_dir) < 0) {
-			bb_perror_msg("chdir(%s) by %s failed", pas->pw_dir, user);
-			xchdir(TMPDIR);
+		fdprintf(fd, "%s\n", pas->pw_name);
+		if (fstat(fd, &st) != 0 || st.st_nlink != 0) {
+			/*close(fd);*/
+			break;
 		}
+		/* st.st_nlink == 0:
+		 * file was deleted, maybe crond missed our notification */
+		close(fd);
+		/* loop */
 	}
-	return pas->pw_uid;
+	if (fd < 0) {
+		bb_error_msg("cannot append to %s/%s",
+				crontab_dir, CRONUPDATE);
+	}
+	return 0;
 }