From: nekral-guest <nekral-guest@5a98b0ae-9ef6-0310-add3-de5d479b70d7>
Date: Sat, 24 May 2008 15:19:02 +0000 (+0000)
Subject: 	* libmisc/fields.c: Avoid assignments in comparisons, assignments
X-Git-Tag: 4.1.2~5
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9c41a8ad38ab58b8c072ef04af9135cac15507f7;p=shadow

	* libmisc/fields.c: Avoid assignments in comparisons, assignments
	with post increments (x++), use of integers as booleans, and
	explicitly mark blocks with brackets.
	* libmisc/copydir.c: Likewise.
	* libmisc/fields.c: Add comments.
	* libmisc/copydir.c: Mark function whose return value is not
	checked as such.

	* libmisc/copydir.c (remove_tree): Make sure unlink is successful
	when removing files.
---

diff --git a/ChangeLog b/ChangeLog
index 9360b09a..2bb4fe6c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2008-05-24  Nicolas François  <nicolas.francois@centraliens.net>
+
+	* libmisc/copydir.c (remove_tree): Make sure unlink is successful
+	when removing files.
+
 2008-05-24  Nicolas François  <nicolas.francois@centraliens.net>
 
 	* libmisc/pwdcheck.c: Simply passwd_check since it's never used
@@ -15,6 +20,11 @@
 	* libmisc/list.c: Avoid assignments in comparisons, assignments
 	with post increments (x++), use of integers as booleans, and
 	explicitly mark blocks with brackets.
+	* libmisc/fields.c: Likewise.
+	* libmisc/copydir.c: Likewise.
+	* libmisc/fields.c: Add comments.
+	* libmisc/copydir.c: Mark function whose return value is not
+	checked as such.
 
 2008-05-23  Nicolas François  <nicolas.francois@centraliens.net>
 
diff --git a/libmisc/copydir.c b/libmisc/copydir.c
index 2435109c..7bbe9684 100644
--- a/libmisc/copydir.c
+++ b/libmisc/copydir.c
@@ -250,7 +250,7 @@ int copy_tree (const char *src_root, const char *dst_root,
 			}
 		}
 	}
-	closedir (dir);
+	(void) closedir (dir);
 
 	if (set_orig) {
 		src_orig = 0;
@@ -412,7 +412,7 @@ static int copy_symlink (const char *src, const char *dst,
 		return -1;
 	}
 	oldlink[len] = '\0';	/* readlink() does not NUL-terminate */
-	if (!strncmp (oldlink, src_orig, strlen (src_orig))) {
+	if (strncmp (oldlink, src_orig, strlen (src_orig)) == 0) {
 		snprintf (dummy, sizeof dummy, "%s%s",
 		          dst_orig,
 		          oldlink + strlen (src_orig));
@@ -533,7 +533,7 @@ static int copy_file (const char *src, const char *dst,
 	               (uid == -1) ? statp->st_uid : (uid_t) uid,
 	               (gid == -1) ? statp->st_gid : (gid_t) gid) != 0)
 	    || (chmod (dst, statp->st_mode & 07777) != 0)) {
-		close (ifd);
+		(void) close (ifd);
 		return -1;
 	}
 
@@ -543,7 +543,7 @@ static int copy_file (const char *src, const char *dst,
 		}
 	}
 
-	close (ifd);
+	(void) close (ifd);
 
 	if (futimes (ofd, mt) != 0) {
 		return -1;
@@ -622,19 +622,22 @@ int remove_tree (const char *root)
 			 * Recursively delete this directory.
 			 */
 
-			if (remove_tree (new_name)) {
+			if (remove_tree (new_name) != 0) {
 				err = -1;
 				break;
 			}
-			if (rmdir (new_name)) {
+			if (rmdir (new_name) != 0) {
+				err = -1;
+				break;
+			}
+		} else {
+			if (unlink (new_name) != 0) {
 				err = -1;
 				break;
 			}
-			continue;
 		}
-		unlink (new_name);
 	}
-	closedir (dir);
+	(void) closedir (dir);
 
 	return err;
 }
diff --git a/libmisc/fields.c b/libmisc/fields.c
index 7eff3ddd..a673659d 100644
--- a/libmisc/fields.c
+++ b/libmisc/fields.c
@@ -38,6 +38,7 @@
 #include <string.h>
 #include <stdio.h>
 #include "prototypes.h"
+
 /*
  * valid_field - insure that a field contains all legal characters
  *
@@ -53,15 +54,22 @@ int valid_field (const char *field, const char *illegal)
 	const char *cp;
 	int err = 0;
 
-	for (cp = field; *cp && !strchr (illegal, *cp); cp++);
-
-	if (*cp) {
-		err = -1;
-	} else {
-		for (cp = field; *cp && isprint (*cp); cp++);
+	/* For each character of field, search if it appears in the list
+	 * of illegal characters. */
+	for (cp = field; '\0' != *cp; cp++) {
+		if (strchr (illegal, *cp) != NULL) {
+			err = -1;
+			break;
+		}
+	}
 
-		if (*cp) {
-			err = 1;
+	if (0 == err) {
+		/* Search if there are some non-printable characters */
+		for (cp = field; '\0' != *cp; cp++) {
+			if (!isprint (*cp)) {
+				err = 1;
+				break;
+			}
 		}
 	}
 
@@ -74,25 +82,28 @@ int valid_field (const char *field, const char *illegal)
  * prompt the user with the name of the field being changed and the
  * current value.
  */
-
 void change_field (char *buf, size_t maxsize, const char *prompt)
 {
 	char newf[200];
 	char *cp;
 
-	if (maxsize > sizeof (newf))
+	if (maxsize > sizeof (newf)) {
 		maxsize = sizeof (newf);
+	}
 
 	printf ("\t%s [%s]: ", prompt, buf);
-	fflush (stdout);
-	if (fgets (newf, maxsize, stdin) != newf)
+	(void) fflush (stdout);
+	if (fgets (newf, (int) maxsize, stdin) != newf) {
 		return;
+	}
 
-	if (!(cp = strchr (newf, '\n')))
+	cp = strchr (newf, '\n');
+	if (NULL == cp) {
 		return;
+	}
 	*cp = '\0';
 
-	if (newf[0]) {
+	if ('\0' != newf[0]) {
 		/*
 		 * Remove leading and trailing whitespace.  This also
 		 * makes it possible to change the field to empty, by
@@ -100,11 +111,13 @@ void change_field (char *buf, size_t maxsize, const char *prompt)
 		 */
 
 		while (--cp >= newf && isspace (*cp));
-		*++cp = '\0';
+		cp++;
+		*cp = '\0';
 
 		cp = newf;
-		while (*cp && isspace (*cp))
+		while (('\0' != *cp) && isspace (*cp)) {
 			cp++;
+		}
 
 		strncpy (buf, cp, maxsize - 1);
 		buf[maxsize - 1] = '\0';