Date: Wed, 8 Aug 2001 17:21:33 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Mark Murray <mark@grondar.za> Cc: <audit@FreeBSD.ORG> Subject: Re: [patch] su(1) WARNS=2 cleanup Message-ID: <20010808165645.W5697-100000@besplex.bde.org> In-Reply-To: <200108071058.f77Aw6Z43835@grimreaper.grondar.za>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010808165645.W5697-100000>