From owner-freebsd-audit  Wed Aug  8  0:25:41 2001
Delivered-To: freebsd-audit@freebsd.org
Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16])
	by hub.freebsd.org (Postfix) with ESMTP id 07AB637B401
	for <audit@FreeBSD.ORG>; Wed,  8 Aug 2001 00:25:37 -0700 (PDT)
	(envelope-from bde@zeta.org.au)
Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102])
	by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id RAA23578;
	Wed, 8 Aug 2001 17:24:02 +1000
Date: Wed, 8 Aug 2001 17:21:33 +1000 (EST)
From: Bruce Evans <bde@zeta.org.au>
X-X-Sender:  <bde@besplex.bde.org>
To: Mark Murray <mark@grondar.za>
Cc: <audit@FreeBSD.ORG>
Subject: Re: [patch] su(1) WARNS=2 cleanup
In-Reply-To: <200108071058.f77Aw6Z43835@grimreaper.grondar.za>
Message-ID: <20010808165645.W5697-100000@besplex.bde.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: owner-freebsd-audit@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-audit.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-audit>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-audit>
X-Loop: FreeBSD.ORG

On Tue, 7 Aug 2001, Mark Murray wrote:

> Please review.
>
> The PAM_SET_ITEM() thing is a separate commit.

> Index: Makefile
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/su/Makefile,v
> retrieving revision 1.32
> diff -u -d -r1.32 Makefile
> --- Makefile	2001/05/26 09:52:36	1.32
> +++ Makefile	2001/08/06 14:04:14
> @@ -5,6 +5,7 @@
>
>  DPADD+= ${LIBUTIL} ${LIBPAM}
>  LDADD+= -lutil ${MINUSLPAM}
> +WARNS?=	2

Style bug (disorder: WARNS added to linker flags "section").

> Index: su.c
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/su/su.c,v
> retrieving revision 1.39
> diff -u -d -r1.39 su.c
> --- su.c	2001/05/26 09:52:36	1.39
> +++ su.c	2001/08/06 14:07:11
> ...
> @@ -213,6 +214,8 @@
>  		errx(1, "pam_start: %s", pam_strerror(pamh, retcode));
>  	}
>
> +	PAM_SET_ITEM(PAM_RUSER, (const void *)getlogin());
> +

Style bugs:
- bogus cast.  pam_set_item(undoc)'s  second parameter has type
  "const void *".  Conversion of getlogin()'s type ("char *") to
  "const void *" is automatic in C.
- excessive vertical whitespace.

> @@ -378,18 +381,19 @@
>
>  		if (iscsh == YES) {
>  			if (fastlogin)
> -				*np-- = "-f";
> +				(const char *)(*np--) = "-f";

Bugs:
- casts are not lvalues in C.
- the cast is to hide the bug that np has the wrong type, but -Wcast-qual
  should warn about it anyway (it doesn't).  The string (pointer) has type
  "const char *", but after assigning it to *np-- it can be accessed via
  *(np + 1) which has type "char *".

>  			if (asme)
> -				*np-- = "-m";
> +				(const char *)(*np--) = "-m";

Bugs, as above.

>  		}
>  		/* csh strips the first character... */
> -		*np = asthem ? "-su" : iscsh == YES ? "_su" : "su";
> +		(const char *)(*np)
> +			= asthem ? "-su" : iscsh == YES ? "_su" : "su";

Bugs, as above.

>
>  		if (ruid != 0)
>  			syslog(LOG_NOTICE, "%s to %s%s", username, user,
>  			    ontty());
>
> -		execv(shell, np);
> +		execv(shell, (char * const *)np);

Style bug: bogus cast.  execve(2)'s second parameter has type
"char * const *".  Conversion of np's type "(char **)" is automatic in C.

Bruce


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message