Bug 12828

Summary: passwd(1) doesn't recognize comments
Product: Base System Reporter: yasuf <yasuf>
Component: binAssignee: Sheldon Hearn <sheldonh>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 4.0-CURRENT   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description yasuf 1999-07-27 03:50:00 UTC 
Passwd(5) manpage says:
|    Lines whose first non-whitespace character is a pound-sign (#) are com-
|    ments, and are ignored.  Blank lines which consist only of spaces, tabs
|    or newlines are also ignored.

but passwd(1) (, chpass(1), etc.) seems not to support these features
correctly.

How-To-Repeat: 
  # tail -3 /etc/master.passwd
  user1:*:9999:9999::0:0:some user:/home/user1:
  # comment
  user2:*:9998:9998::0:0:other user:/home/user2:

  # passwd user1
  Changing local password for user1.
  New password:
  Retype new password:
  passwd: updating the database...
  passwd: done

  # passwd user2
  Changing local password for user2.
  New password:
  Retype new password:
  passwd: /etc/master.passwd: corrupted entry
  passwd: /etc/master.passwd: unchanged
Comment 1 Sheldon Hearn freebsd_committer freebsd_triage 1999-07-27 10:48:34 UTC 
Responsible Changed
From-To: freebsd-bugs->sheldonh

You're right. vipw handles comments and blank lines correctly, but  
passwd does not. I'll take a look.
Comment 2 Sheldon Hearn 1999-07-28 12:07:52 UTC 
On Tue, 27 Jul 1999 02:49:49 MST, sheldonh@FreeBSD.org wrote:

> Responsible-Changed-Why: 
> You're right. vipw handles comments and blank lines correctly, but 
> passwd does not. I'll take a look.

Your patch works. I like the fact that you take care to maintain the
optimization centered around (done). I'd prefer the following, though.
Happy with it?

Ciao,
Sheldon.

Index: pw_copy.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/chpass/pw_copy.c,v
retrieving revision 1.7
diff -u -d -r1.7 pw_copy.c
--- pw_copy.c	1999/04/25 22:37:56	1.7
+++ pw_copy.c	1999/07/28 11:02:37
@@ -40,6 +40,7 @@
  * record, by chpass(1) and passwd(1).
  */
 
+#include <ctype.h>
 #include <err.h>
 #include <pwd.h>
 #include <stdio.h>
@@ -80,6 +81,14 @@
 			pw_error(NULL, 0, 1);
 		}
 		if (done) {
+			(void)fprintf(to, "%s", buf);
+			if (ferror(to))
+				goto err;
+			continue;
+		}
+		for (p = buf; isspace(*p); p++)
+			;	/* nothing */
+		if (*p == '#' || *p == '\0') {
 			(void)fprintf(to, "%s", buf);
 			if (ferror(to))
 				goto err;
Comment 3 Sheldon Hearn 1999-07-29 11:28:54 UTC 
While checking for the performance hit of my code on a 100000 entry
passwd file, I noticed pwd_mkdb is also broken with respect to the
passwd(5) manpage. There may be others.

Expect delays, not because I'm not looking at it, but because I'd prefer
to get everything inline at once, having done some decent testing
and with a unified review. I don't want piecemeal commits on this if
possible.

Ciao,
Sheldon.
Comment 4 Sheldon Hearn 1999-07-29 15:22:22 UTC 
On Thu, 29 Jul 1999 12:28:54 +0200, Sheldon Hearn wrote:

> While checking for the performance hit of my code on a 100000 entry
> passwd file, [...]

I must have made some peculiar mistake when doing my first lot of tests.
The patch on this PR shows no significant performance hit on a 50000
entry passwd file. Therefore, the complexity of the alternative that I
came up with (not posted to the PR) is unjustified.

I'll post a patch for pwd_mkdb when it's done.

Ciao,
Sheldon.
Comment 5 Sheldon Hearn freebsd_committer freebsd_triage 1999-07-29 17:37:46 UTC 
State Changed
From-To: open->closed

I've used your patch after all, since it matches more closely the code 
used in pwd_mkdb.c . Can you believe that we support usernames with spaces 
in them? :-)    I haven't touched pwd_mkdb, since the USHRT_MAX  
comparison generates warnings, not failures.