svn commit: r305821 - head/usr.bin/login

[Bruce Evans]

On Thu, 15 Sep 2016, Ed Maste wrote:

> Log:
>  login: clean up errx strings
>
>  errx() prefixes the error string with argv[0] so including "login: "
>  in the string is redundant. Also remove a superfluous newline.

The strings still have plenty of dirt:

> Modified: head/usr.bin/login/login_audit.c
> ==============================================================================
> --- head/usr.bin/login/login_audit.c	Wed Sep 14 23:24:23 2016	(r305820)
> +++ head/usr.bin/login/login_audit.c	Thu Sep 15 01:55:18 2016	(r305821)
> @@ -73,14 +73,14 @@ au_login_success(void)
>  	if (auditon(A_GETCOND, &au_cond, sizeof(au_cond)) < 0) {
> 		if (errno == ENOSYS)
> 			return;
> -		errx(1, "login: Could not determine audit condition");
> +		errx(1, "Could not determine audit condition");

Capitalization.  Error messages are conventionally not capitalized
(but strerror() strings break this).  This file does this in many
more places, but not all.

> @@ -88,22 +88,22 @@ au_login_success(void)
> 	bcopy(&tid, &auinfo.ai_termid, sizeof(auinfo.ai_termid));
> 	bcopy(&aumask, &auinfo.ai_mask, sizeof(auinfo.ai_mask));
> 	if (setaudit(&auinfo) != 0)
> -		err(1, "login: setaudit failed");
> +		err(1, "setaudit failed");

Example of normal style for an error message.  Should possibly indicate
that it was a specific function/syscall that failed by marking up the
function with "()".

>
> 	if ((aufd = au_open()) == -1)
> -		errx(1,"login: Audit Error: au_open() failed");
> +		errx(1, "Audit Error: au_open() failed");

This marks up the function and adds the doubly Capitalized spam
"Audit Error: ".  Who would have thought that an error message is for
an error?  "Audit " is not as redundant as that, but is not used in
all of the messages.

Since this uses errx() and not err(), a different style is justified,
but it is probably wrong to not use strerror() when a function/syscall
name is printed.  The "failed" part of the message is nondescript.

>
> 	if ((tok = au_to_subject32(uid, geteuid(), getegid(), uid, gid, pid,
> 	    pid, &tid)) == NULL)
> -		errx(1, "login: Audit Error: au_to_subject32() failed");
> +		errx(1, "Audit Error: au_to_subject32() failed");
> 	au_write(aufd, tok);
>
> 	if ((tok = au_to_return32(0, 0)) == NULL)
> -		errx(1, "login: Audit Error: au_to_return32() failed");
> +		errx(1, "Audit Error: au_to_return32() failed");
> 	au_write(aufd, tok);

Spam continues.  The function name is a bit too long to print.

>
> 	if (au_close(aufd, 1, AUE_login) == -1)
> -		errx(1, "login: Audit Record was not committed.");
> +		errx(1, "Audit Record was not committed.");
> }

Now "Audit Record" is useful because the function name is not printed.
Not printing the function name is a different and perhaps better style.
I usually print the function name when I don't want to think about a
better discription.

Error messages are conventionally not capitalized, and most in this file
are not. but this one is.

>
> /*
> @@ -124,13 +124,13 @@ au_login_fail(const char *errmsg, int na
>  	if (auditon(A_GETCOND, &au_cond, sizeof(au_cond)) < 0) {
> 		if (errno == ENOSYS)
> 			return;
> -		errx(1, "login: Could not determine audit condition");
> +		errx(1, "Could not determine audit condition");
> 	}

Back to only 1 capitialization error.

> 	if (au_cond == AUC_NOAUDIT)
> 		return;
>
> 	if ((aufd = au_open()) == -1)
> -		errx(1, "login: Audit Error: au_open() failed");
> +		errx(1, "Audit Error: au_open() failed");

Back to spam.

>
> 	if (na) {
> 		/*
> @@ -139,28 +139,28 @@ au_login_fail(const char *errmsg, int na
> 		 */
> 		if ((tok = au_to_subject32(-1, geteuid(), getegid(), -1, -1,
> 		    pid, -1, &tid)) == NULL)
> -			errx(1, "login: Audit Error: au_to_subject32() failed");
> +			errx(1, "Audit Error: au_to_subject32() failed");
> 	} else {
> 		/* We know the subject -- so use its value instead. */
> 		uid = pwd->pw_uid;
> 		gid = pwd->pw_gid;
> 		if ((tok = au_to_subject32(uid, geteuid(), getegid(), uid,
> 		    gid, pid, pid, &tid)) == NULL)
> -			errx(1, "login: Audit Error: au_to_subject32() failed");
> +			errx(1, "Audit Error: au_to_subject32() failed");
> ...

The general style reduces to "error: xxx failed [duh]" with careful use
of errx() instead of err() to prevent leakage of useful information.

Bruce