Clean up error handling in fileupdate(), which now returns 0 on success

instead of a boolean. This replicated through he front-end sub-functions
relating to add, delete, modify entries in passwd & group files
Errno is now preserved so output of errc()/warnc() will be less obfuscated
by subsequent errors when reporting the problem.

Add more intelligent error handling when attempting to modify/delete NIS
entries with no corresponding local database entry.

[MFC to stable in a couple of weeks to keep both in sync]

5 files changed, 117 insertions, 62 deletions

diff --git a/pw/fileupd.c b/pw/fileupd.c
index 376f589..a846513 100644
--- a/pw/fileupd.c
+++ b/pw/fileupd.c
@@ -71,38 +71,44 @@ extendarray(char ***buf, int * buflen, int needed)
 int
 fileupdate(char const * filename, mode_t fmode, char const * newline, char const * prefix, int pfxlen, int updmode)
 {
-	int             rc = 0;
+	int     rc = 0;
 
 	if (pfxlen <= 1)
-		errno = EINVAL;
+		rc = EINVAL;
 	else {
-		int             infd = open(filename, O_RDWR | O_CREAT, fmode);
+		int    infd = open(filename, O_RDWR | O_CREAT, fmode);
 
-		if (infd != -1) {
-			FILE           *infp = fdopen(infd, "r+");
+		if (infd == -1)
+			rc = errno;
+		else {
+			FILE   *infp = fdopen(infd, "r+");
 
-			if (infp == NULL)
+			if (infp == NULL) {
+				rc = errno;		/* Assumes fopen(3) sets errno from open(2) */
 				close(infd);
-			else {
-				int             outfd;
-				char            file[MAXPATHLEN];
+			} else {
+				int       outfd;
+				char      file[MAXPATHLEN];
 
 				strcpy(file, filename);
 				strcat(file, ".new");
 				outfd = open(file, O_RDWR | O_CREAT | O_TRUNC | O_EXLOCK, fmode);
-				if (outfd != -1) {
-					FILE           *outfp = fdopen(outfd, "w+");
+				if (outfd == -1)
+					rc = errno;
+				else {
+					FILE    *outfp = fdopen(outfd, "w+");
 
-					if (outfp == NULL)
+					if (outfp == NULL) {
+						rc = errno;
 						close(outfd);
-					else {
-						int             updated = UPD_CREATE;
+					} else {
+						int   updated = UPD_CREATE;
 						int		linesize = PWBUFSZ;
-						char           *line = malloc(linesize);
+						char  *line = malloc(linesize);
 
 					nextline:
 						while (fgets(line, linesize, infp) != NULL) {
-							char           *p = strchr(line, '\n');
+							char  *p = strchr(line, '\n');
 
 							while ((p = strchr(line, '\n')) == NULL) {
 								int	l;
@@ -143,7 +149,11 @@ fileupdate(char const * filename, mode_t fmode, char const * newline, char const
 						 * then error
 						 */
 						if (updmode != updated)
-							errno = (updmode == UPD_CREATE) ? EEXIST : ENOENT;
+							/* -1 return means:
+							 * update,delete=no user entry
+							 * create=entry exists
+							 */
+							rc = -1;
 						else {
 
 							/*
@@ -155,9 +165,9 @@ fileupdate(char const * filename, mode_t fmode, char const * newline, char const
 							/*
 							 * Flush the file and check for the result
 							 */
-							rc = fflush(outfp) != EOF;
-							if (rc) {
-
+							if (fflush(outfp) == EOF)
+								rc = errno;	/* Failed to update */
+							else {
 								/*
 								 * Copy data back into the
 								 * original file and truncate
@@ -168,18 +178,16 @@ fileupdate(char const * filename, mode_t fmode, char const * newline, char const
 									fputs(line, infp);
 
 								/*
-                                                                 * If there was a problem with copying
-                                                                 * we will just rename 'file.new' 
-                                                                 * to 'file'.
+								 * If there was a problem with copying
+								 * we will just rename 'file.new' 
+								 * to 'file'.
 								 * This is a gross hack, but we may have
 								 * corrupted the original file
 								 * Unfortunately, it will lose the inode
-                                                                 * and hence the lock.
+								 * and hence the lock.
 								 */
-								if (fflush(infp) == EOF || ferror(infp)) {
-									rc = errno;	/* Preserve errno for return */
+								if (fflush(infp) == EOF || ferror(infp))
 									rename(file, filename);
-								}
 								else
 									ftruncate(infd, ftell(infp));
 							}
diff --git a/pw/grupd.c b/pw/grupd.c
index 237fe95..edff76d 100644
--- a/pw/grupd.c
+++ b/pw/grupd.c
@@ -142,8 +142,9 @@ gr_update(struct group * grp, char const * group, int mode)
 	if (grp != NULL && fmtgrentry(&grbuf, &grbuflen, grp, PWF_PASSWD) == -1)
 		l = -1;
 	else {
-		if ((l = fileupdate(getgrpath(_GROUP), 0644, grbuf, pfx, l, mode)) != 0)
-			l = grdb(NULL) == 0;
+		l = fileupdate(getgrpath(_GROUP), 0644, grbuf, pfx, l, mode);
+		if (l == 0)
+			l = grdb(NULL);
 	}
 	if (grbuf != NULL)
 		free(grbuf);
diff --git a/pw/pw_group.c b/pw/pw_group.c
index 3385752..54125d8 100644
--- a/pw/pw_group.c
+++ b/pw/pw_group.c
@@ -44,12 +44,13 @@ static gid_t    gr_gidpolicy(struct userconf * cnf, struct cargs * args);
 int
 pw_group(struct userconf * cnf, int mode, struct cargs * args)
 {
+	int		rc;
 	struct carg    *a_name = getarg(args, 'n');
 	struct carg    *a_gid = getarg(args, 'g');
 	struct carg    *arg;
 	struct group   *grp = NULL;
 	int	        grmembers = 0;
-	char          **members = NULL;
+	char           **members = NULL;
 
 	static struct group fakegroup =
 	{
@@ -116,8 +117,13 @@ pw_group(struct userconf * cnf, int mode, struct cargs * args)
 		if (mode == M_DELETE) {
 			gid_t           gid = grp->gr_gid;
 
-			if (delgrent(grp) == -1)
-				err(EX_IOERR, "error updating group file");
+			rc = delgrent(grp);
+			if (rc == -1)
+				err(EX_IOERR, "group '%s' not available (NIS?)", grp->gr_name);
+			else if (rc != 0) {
+				warnc(rc, "group update");
+				return EX_IOERR;
+			}
 			pw_log(cnf, mode, W_GROUP, "%s(%ld) removed", a_name->val, (long) gid);
 			return EXIT_SUCCESS;
 		} else if (mode == M_PRINT)
@@ -231,8 +237,17 @@ pw_group(struct userconf * cnf, int mode, struct cargs * args)
 	if (getarg(args, 'N') != NULL)
 		return print_group(grp, getarg(args, 'P') != NULL);
 
-	if ((mode == M_ADD && !addgrent(grp)) || (mode == M_UPDATE && !chggrent(a_name->val, grp))) {
-		warn("group update");
+	if (mode == M_ADD && (rc = addgrent(grp)) != 0) {
+		if (rc == -1)
+			warnx("group '%s' already exists", grp->gr_name);
+		else
+			warn("group update");
+		return EX_IOERR;
+	} else if (mode == M_UPDATE && (rc = chggrent(a_name->val, grp)) != 0) {
+		if (rc == -1)
+			warnx("group '%s' not available (NIS?)", grp->gr_name);
+		else
+			warnc(rc, "group update");
 		return EX_IOERR;
 	}
 	/* grp may have been invalidated */
diff --git a/pw/pw_user.c b/pw/pw_user.c
index bba8a01..da6c83b 100644
--- a/pw/pw_user.c
+++ b/pw/pw_user.c
@@ -102,7 +102,7 @@ static void	rmskey(char const * name);
 int
 pw_user(struct userconf * cnf, int mode, struct cargs * args)
 {
-	int	        r, r1;
+	int	        rc;
 	char           *p = NULL;
 	struct carg    *a_name;
 	struct carg    *a_uid;
@@ -357,12 +357,23 @@ pw_user(struct userconf * cnf, int mode, struct cargs * args)
 			strncpy(home, pwd->pw_dir, sizeof home);
 			home[sizeof home - 1] = '\0';
 
-			if (!delpwent(pwd))
-				err(EX_IOERR, "error updating passwd file");
+			rc = delpwent(pwd);
+			if (rc == -1)
+				err(EX_IOERR, "user '%s' does not exist", pwd->pw_name);
+			else if (rc != 0) {
+				warnc(rc, "passwd update");
+				return EX_IOERR;
+			}
+
+			if (cnf->nispasswd && *cnf->nispasswd=='/') {
+				rc = delnispwent(cnf->nispasswd, a_name->val);
+				if (rc == -1)
+					warnx("WARNING: user '%s' does not exist in NIS passwd", pwd->pw_name);
+				else if (rc != 0)
+					warnc(rc, "WARNING: NIS passwd update");
+				/* non-fatal */
+			}
 
-			if (cnf->nispasswd && *cnf->nispasswd=='/' && !delnispwent(cnf->nispasswd, a_name->val))
-				warn("WARNING: NIS passwd update");
-				
 			editgroups(a_name->val, NULL);
 
 			pw_log(cnf, mode, W_USER, "%s(%ld) account removed", a_name->val, (long) uid);
@@ -535,23 +546,40 @@ pw_user(struct userconf * cnf, int mode, struct cargs * args)
 				  getarg(args, 'P') != NULL,
 				  getarg(args, '7') != NULL);
 
-	r = r1 = 1;
 	if (mode == M_ADD) {
-		r = addpwent(pwd);
-		if (r && cnf->nispasswd && *cnf->nispasswd=='/')
-			r1 = addnispwent(cnf->nispasswd, pwd);
+		rc = addpwent(pwd);
+		if (rc == -1) {
+			warnx("user '%s' already exists", pwd->pw_name);
+			return EX_IOERR;
+		} else if (rc != 0) {
+			warnc(rc, "passwd file update");
+			return EX_IOERR;
+		}
+		if (cnf->nispasswd && *cnf->nispasswd=='/') {
+			rc = addnispwent(cnf->nispasswd, pwd);
+			if (rc == -1)
+				warnx("User '%s' already exists in NIS passwd", pwd->pw_name);
+			else
+				warnc(rc, "NIS passwd update");
+			/* NOTE: we treat NIS-only update errors as non-fatal */
+		}
 	} else if (mode == M_UPDATE) {
-		r = chgpwent(a_name->val, pwd);
-		if (r && cnf->nispasswd && *cnf->nispasswd=='/')
-			r1 = chgnispwent(cnf->nispasswd, a_name->val, pwd);
-	}
-
-	if (!r) {
-		warn("password update");
-		return EX_IOERR;
-	} else if (!r1) {
-		warn("WARNING: NIS password update");
-		/* Keep on trucking */
+		rc = chgpwent(a_name->val, pwd);
+		if (rc == -1) {
+			warnx("user '%s' does not exist (NIS?)", pwd->pw_name);
+			return EX_IOERR;
+		} else if (rc != 0) {
+			warnc(rc, "passwd file update");
+			return EX_IOERR;
+		}
+		if ( cnf->nispasswd && *cnf->nispasswd=='/') {
+			rc = chgnispwent(cnf->nispasswd, a_name->val, pwd);
+			if (rc == -1)
+				warn("User '%s' not found in NIS passwd", pwd->pw_name);
+			else
+				warnc(rc, "NIS passwd update");
+			/* NOTE: NIS-only update errors are not fatal */
+		}
 	}
 
 	/*
diff --git a/pw/pwupd.c b/pw/pwupd.c
index 10a6066..baaf102 100644
--- a/pw/pwupd.c
+++ b/pw/pwupd.c
@@ -92,14 +92,14 @@ pwdb(char *arg,...)
 	args[i] = NULL;
 
 	if ((pid = fork()) == -1)	/* Error (errno set) */
-		i = -1;
+		i = errno;
 	else if (pid == 0) {	/* Child */
 		execv(args[0], args);
 		_exit(1);
 	} else {		/* Parent */
 		waitpid(pid, &i, 0);
-		if ((i = WEXITSTATUS(i)) != 0)
-			errno = EIO;	/* set SOMETHING */
+		if (WEXITSTATUS(i))
+			i = EIO;
 	}
 	return i;
 }
@@ -161,18 +161,21 @@ pw_update(struct passwd * pwd, char const * user, int mode)
 			*pwbuf = '\0';
 		else
 			fmtpwentry(pwbuf, pwd, PWF_PASSWD);
-		if ((rc = fileupdate(getpwpath(_PASSWD), 0644, pwbuf, pfx, l, mode)) != 0) {
+
+		rc = fileupdate(getpwpath(_PASSWD), 0644, pwbuf, pfx, l, mode);
+		if (rc == 0) {
 
 			/*
 			 * Then the master.passwd file
 			 */
 			if (pwd != NULL)
 				fmtpwentry(pwbuf, pwd, PWF_MASTER);
-			if ((rc = fileupdate(getpwpath(_MASTERPASSWD), 0644, pwbuf, pfx, l, mode)) != 0) {
+			rc = fileupdate(getpwpath(_MASTERPASSWD), 0644, pwbuf, pfx, l, mode);
+			if (rc != 0) {
 				if (mode == UPD_DELETE)
-					rc = pwdb(NULL) == 0;
+					rc = pwdb(NULL);
 				else
-					rc = pwdb("-u", user, NULL) == 0;
+					rc = pwdb("-u", user, NULL);
 			}
 		}
 	}