Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 11 Jan 1997 12:39:07 -0700 (MST)
From:      Terry Lambert <terry@lambert.org>
To:        joerg_wunsch@uriah.heep.sax.de
Cc:        hackers@freebsd.org
Subject:   Re: unused variable in su
Message-ID:  <199701111939.MAA24005@phaeton.artisoft.com>
In-Reply-To: <Mutt.19970111131424.j@uriah.heep.sax.de> from "J Wunsch" at Jan 11, 97 01:14:24 pm

next in thread | previous in thread | raw e-mail | index | archive | help
> Still, it's fairly obfuscated code.  It could be better worded:
> 
> 	if (asme)
> 		if (pwd->pw_shell && *pwd->pw_shell) {
> 			(void)strcpy(shellbuf,  pwd->pw_shell);
> 			shell = shellbuf;
> 		} else {
> 			shell = _PATH_BSHELL;
> 			iscsh = NO;
> 		}
> 
> This would be less confusing for compilers and human readers.

Human readers are expected to have familiarity with the return
values of standard library routines.  Explicitly void casting
strcpy indicates it has a return value which is being ignored,
for no good reason.

> Btw., shouldn't it better be a strncpy() anyway?  Sure, /etc/shells is
> at the mercy of the sysadmin, but he isn't unfailable.

Personally, I'd use strdup() istead of an auto buffer to allocate
the buffer at whatever size is necessary... ie: get rid of shellbuf
entirely and replace:

 			shell = strcpy(shellbuf,  pwd->pw_shell);
with:
 			shell = strdup( pwd->pw_shell);

But, hey, that's me, using strdup() for what it was intended to do.

8-).


					Terry Lambert
					terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199701111939.MAA24005>