CFR: Exceedingly minor fixes to libc

From: Garrett Wollman <wollman_at_bimajority.org>
Date: Fri, 13 Nov 2009 00:18:49 -0500
If you have a moment, please take a look at the following patch.  It
contains some very minor fixes to various parts of libc which were
found by the clang static analyzer.  They fall into a few categories:

1) Bug fixes in very rare situations (mostly error-handling code that
has probably never been executed).

2) Dead store elimination.

3) Elimination of unused variables.  (Or in a few cases, making use of
them.)

Some minor style problems were also fixed in the vicinity.  There
should be no functional changes except in very unusual conditions.

-GAWollman


Index: stdio/fvwrite.c
===================================================================
--- stdio/fvwrite.c	(revision 199242)
+++ stdio/fvwrite.c	(working copy)
_at__at_ -60,7 +60,7 _at__at_
 	char *nl;
 	int nlknown, nldist;
 
-	if ((len = uio->uio_resid) == 0)
+	if (uio->uio_resid == 0)
 		return (0);
 	/* make sure we can write */
 	if (prepwrite(fp) != 0)
Index: stdio/vfwprintf.c
===================================================================
--- stdio/vfwprintf.c	(revision 199242)
+++ stdio/vfwprintf.c	(working copy)
_at__at_ -293,7 +293,7 _at__at_
 		 * number of characters to print.
 		 */
 		p = mbsarg;
-		insize = nchars = 0;
+		insize = nchars = nconv = 0;
 		mbs = initial_mbs;
 		while (nchars != (size_t)prec) {
 			nconv = mbrlen(p, MB_CUR_MAX, &mbs);
Index: stdio/xprintf_time.c
===================================================================
--- stdio/xprintf_time.c	(revision 199242)
+++ stdio/xprintf_time.c	(working copy)
_at__at_ -64,7 +64,6 _at__at_
 	intmax_t t, tx;
 	int i, prec, nsec;
 
-	prec = 0;
 	if (pi->is_long) {
 		tv = *((struct timeval **)arg[0]);
 		t = tv->tv_sec;
_at__at_ -78,6 +77,8 _at__at_
 	} else {
 		tp = *((time_t **)arg[0]);
 		t = *tp;
+		nsec = 0;
+		prec = 0;
 	}
 
 	p = buf;
Index: stdio/fgetws.c
===================================================================
--- stdio/fgetws.c	(revision 199242)
+++ stdio/fgetws.c	(working copy)
_at__at_ -89,7 +89,7 _at__at_
 	if (!__mbsinit(&fp->_mbstate))
 		/* Incomplete character */
 		goto error;
-	*wsp++ = L'\0';
+	*wsp = L'\0';
 	FUNLOCKFILE(fp);
 
 	return (ws);
Index: rpc/getnetconfig.c
===================================================================
--- rpc/getnetconfig.c	(revision 199242)
+++ rpc/getnetconfig.c	(working copy)
_at__at_ -412,13 +412,13 _at__at_
      * Noone needs these entries anymore, then frees them.
      * Make sure all info in netconfig_info structure has been reinitialized.
      */
-    q = p = ni.head;
+    q = ni.head;
     ni.eof = ni.ref = 0;
     ni.head = NULL;
     ni.tail = NULL;
     mutex_unlock(&ni_lock);
 
-    while (q) {
+    while (q != NULL) {
 	p = q->next;
 	if (q->ncp->nc_lookups != NULL) free(q->ncp->nc_lookups);
 	free(q->ncp);
Index: rpc/svc_raw.c
===================================================================
--- rpc/svc_raw.c	(revision 199242)
+++ rpc/svc_raw.c	(working copy)
_at__at_ -176,9 +176,8 _at__at_
 		msg->acpted_rply.ar_results.proc = (xdrproc_t) xdr_void;
 		msg->acpted_rply.ar_results.where = NULL;
 
-		if (!xdr_replymsg(xdrs, msg) ||
-		    !SVCAUTH_WRAP(&SVC_AUTH(xprt), xdrs, xdr_proc, xdr_where))
-			stat = FALSE;
+		stat = xdr_replymsg(xdrs, msg) &&
+		    SVCAUTH_WRAP(&SVC_AUTH(xprt), xdrs, xdr_proc, xdr_where);
 	} else {
 		stat = xdr_replymsg(xdrs, msg);
 	}
Index: rpc/clnt_raw.c
===================================================================
--- rpc/clnt_raw.c	(revision 199242)
+++ rpc/clnt_raw.c	(working copy)
_at__at_ -92,13 +92,13 _at__at_
 	rpcprog_t prog;
 	rpcvers_t vers;
 {
-	struct clntraw_private *clp = clntraw_private;
+	struct clntraw_private *clp;
 	struct rpc_msg call_msg;
-	XDR *xdrs = &clp->xdr_stream;
-	CLIENT	*client = &clp->client_object;
+	XDR *xdrs;
+	CLIENT	*client;
 
 	mutex_lock(&clntraw_lock);
-	if (clp == NULL) {
+	if ((clp = clntraw_private) == NULL) {
 		clp = (struct clntraw_private *)calloc(1, sizeof (*clp));
 		if (clp == NULL) {
 			mutex_unlock(&clntraw_lock);
_at__at_ -110,6 +110,9 _at__at_
 		clp->_raw_buf = __rpc_rawcombuf;
 		clntraw_private = clp;
 	}
+	xdrs = &clp->xdr_stream;
+	client = &clp->client_object;
+
 	/*
 	 * pre-serialize the static part of the call msg and stash it away
 	 */
Index: rpc/svc_vc.c
===================================================================
--- rpc/svc_vc.c	(revision 199242)
+++ rpc/svc_vc.c	(working copy)
_at__at_ -141,7 +141,7 _at__at_
 	r = mem_alloc(sizeof(*r));
 	if (r == NULL) {
 		warnx("svc_vc_create: out of memory");
-		goto cleanup_svc_vc_create;
+		return NULL;
 	}
 	r->sendsize = __rpc_get_t_size(si.si_af, si.si_proto, (int)sendsize);
 	r->recvsize = __rpc_get_t_size(si.si_af, si.si_proto, (int)recvsize);
Index: rpc/getrpcent.c
===================================================================
--- rpc/getrpcent.c	(revision 199242)
+++ rpc/getrpcent.c	(working copy)
_at__at_ -698,7 +698,7 _at__at_
 		return (NS_RETURN);
 	}
 
-	memcpy(&new_rpc, rpc, sizeof(struct rpcent));
+	new_rpc = *rpc;
 
 	*buffer_size = desired_size;
 	memset(buffer, 0, desired_size);
Index: rpc/rpcb_st_xdr.c
===================================================================
--- rpc/rpcb_st_xdr.c	(revision 199242)
+++ rpc/rpcb_st_xdr.c	(working copy)
_at__at_ -89,7 +89,6 _at__at_
 	rpcbs_rmtcalllist *objp;
 {
 	int32_t *buf;
-	struct rpcbs_rmtcalllist **pnext;
 
 	if (xdrs->x_op == XDR_ENCODE) {
 	buf = XDR_INLINE(xdrs, 6 * BYTES_PER_XDR_UNIT);
_at__at_ -123,8 +122,7 _at__at_
 	if (!xdr_string(xdrs, &objp->netid, (u_int)~0)) {
 		return (FALSE);
 	}
-	pnext = &objp->next;
-	if (!xdr_pointer(xdrs, (char **) pnext,
+	if (!xdr_pointer(xdrs, (char **) &objp->next,
 			sizeof (rpcbs_rmtcalllist),
 			(xdrproc_t)xdr_rpcbs_rmtcalllist)) {
 		return (FALSE);
_at__at_ -162,7 +160,7 _at__at_
 	if (!xdr_string(xdrs, &objp->netid, (u_int)~0)) {
 		return (FALSE);
 	}
-	if (!xdr_pointer(xdrs, (char **) pnext,
+	if (!xdr_pointer(xdrs, (char **) &objp->next,
 			sizeof (rpcbs_rmtcalllist),
 			(xdrproc_t)xdr_rpcbs_rmtcalllist)) {
 		return (FALSE);
_at__at_ -190,7 +188,7 _at__at_
 	if (!xdr_string(xdrs, &objp->netid, (u_int)~0)) {
 		return (FALSE);
 	}
-	if (!xdr_pointer(xdrs, (char **) pnext,
+	if (!xdr_pointer(xdrs, (char **) &objp->next,
 			sizeof (rpcbs_rmtcalllist),
 			(xdrproc_t)xdr_rpcbs_rmtcalllist)) {
 		return (FALSE);
Index: rpc/key_call.c
===================================================================
--- rpc/key_call.c	(revision 199242)
+++ rpc/key_call.c	(working copy)
_at__at_ -302,7 +302,7 _at__at_
 	void *localhandle;
 	struct netconfig *nconf;
 	struct netconfig *tpconf;
-	struct key_call_private *kcp = key_call_private_main;
+	struct key_call_private *kcp;
 	struct timeval wait_time;
 	struct utsname u;
 	int main_thread;
Index: yp/yplib.c
===================================================================
--- yp/yplib.c	(revision 199242)
+++ yp/yplib.c	(working copy)
_at__at_ -241,7 +241,7 _at__at_
 ypmatch_cache_lookup(struct dom_binding *ypdb, char *map, keydat *key,
     valdat *val)
 {
-	struct ypmatch_ent	*c = ypdb->cache;
+	struct ypmatch_ent	*c;
 
 	ypmatch_cache_expire(ypdb);
 
Index: gen/setmode.c
===================================================================
--- gen/setmode.c	(revision 199242)
+++ gen/setmode.c	(working copy)
_at__at_ -86,7 +86,7 _at__at_
 
 	set = (const BITCMD *)bbox;
 	newmode = omode;
-	for (value = 0;; set++)
+	for (;; set++)
 		switch(set->cmd) {
 		/*
 		 * When copying the user, group or other bits around, we "know"
_at__at_ -147,6 +147,7 _at__at_
 }
 
 #define	ADDCMD(a, b, c, d)						\
+    do {								\
 	if (set >= endset) {						\
 		BITCMD *newset;						\
 		setlen += SET_LEN_INCR;					\
_at__at_ -161,7 +162,8 _at__at_
 		saveset = newset;					\
 		endset = newset + (setlen - 2);				\
 	}								\
-	set = addcmd(set, (a), (b), (c), (d))
+	set = addcmd(set, (a), (b), (c), (d));				\
+    } while (0)
 
 #define	STANDARD_BITS	(S_ISUID|S_ISGID|S_IRWXU|S_IRWXG|S_IRWXO)
 
_at__at_ -173,11 +175,12 _at__at_
 	BITCMD *set, *saveset, *endset;
 	sigset_t sigset, sigoset;
 	mode_t mask;
-	int equalopdone=0, permXbits, setlen;
+	int equalopdone, permXbits, setlen;
 	long perml;
 
 	if (!*p)
 		return (NULL);
+	equalopdone = 0;
 
 	/*
 	 * Get a copy of the mask for the permissions that are mask relative.
_at__at_ -299,16 +302,10 _at__at_
 				 * Add any permissions that we haven't already
 				 * done.
 				 */
-				if (perm || (op == '=' && !equalopdone)) {
-					if (op == '=')
-						equalopdone = 1;
+				if (perm || (op == '=' && !equalopdone))
 					ADDCMD(op, who, perm, mask);
-					perm = 0;
-				}
-				if (permXbits) {
+				if (permXbits)
 					ADDCMD('X', who, permXbits, mask);
-					permXbits = 0;
-				}
 				goto apply;
 			}
 		}
Index: gen/wordexp.c
===================================================================
--- gen/wordexp.c	(revision 199242)
+++ gen/wordexp.c	(working copy)
_at__at_ -320,7 +320,7 _at__at_
 				if (c == '\0' || level != 0)
 					return (WRDE_SYNTAX);
 			} else
-				c = *--words;
+				--words;
 			break;
 		default:
 			break;
Index: gen/getcap.c
===================================================================
--- gen/getcap.c	(revision 199242)
+++ gen/getcap.c	(working copy)
_at__at_ -647,7 +647,7 _at__at_
 cgetnext(char **bp, char **db_array)
 {
 	size_t len;
-	int done, hadreaderr, i, savederrno, status;
+	int done, hadreaderr, savederrno, status;
 	char *cp, *line, *rp, *np, buf[BSIZE], nbuf[BSIZE];
 	u_int dummy;
 
_at__at_ -658,7 +658,7 _at__at_
 		(void)cgetclose();
 		return (-1);
 	}
-	for(;;) {
+	for (;;) {
 		if (toprec && !gottoprec) {
 			gottoprec = 1;
 			line = toprec;
_at__at_ -709,7 +709,6 _at__at_
 		/*
 		 * Line points to a name line.
 		 */
-		i = 0;
 		done = 0;
 		np = nbuf;
 		for (;;) {
Index: gen/getpwent.c
===================================================================
--- gen/getpwent.c	(revision 199242)
+++ gen/getpwent.c	(working copy)
_at__at_ -257,22 +257,19 _at__at_
 pwd_marshal_func(char *buffer, size_t *buffer_size, void *retval, va_list ap,
     void *cache_mdata)
 {
-	char *name;
-	uid_t uid;
 	struct passwd *pwd;
-	char *orig_buf;
-	size_t orig_buf_size;
 
 	struct passwd new_pwd;
 	size_t desired_size, size;
 	char *p;
 
+	/* advance past unused arguments */
 	switch ((enum nss_lookup_type)cache_mdata) {
 	case nss_lt_name:
-		name = va_arg(ap, char *);
+		va_arg(ap, char *);
 		break;
 	case nss_lt_id:
-		uid = va_arg(ap, uid_t);
+		va_arg(ap, uid_t);
 		break;
 	case nss_lt_all:
 		break;
_at__at_ -282,8 +279,7 _at__at_
 	}
 
 	pwd = va_arg(ap, struct passwd *);
-	orig_buf = va_arg(ap, char *);
-	orig_buf_size = va_arg(ap, size_t);
+	va_end(ap);			/* remaining args are unused */
 
 	desired_size = sizeof(struct passwd) + sizeof(char *) +
 	    strlen(pwd->pw_name) + 1;
_at__at_ -361,8 +357,6 _at__at_
 pwd_unmarshal_func(char *buffer, size_t buffer_size, void *retval, va_list ap,
     void *cache_mdata)
 {
-	char *name;
-	uid_t uid;
 	struct passwd *pwd;
 	char *orig_buf;
 	size_t orig_buf_size;
_at__at_ -370,12 +364,13 _at__at_
 
 	char *p;
 
+	/* advance past unused arguments */
 	switch ((enum nss_lookup_type)cache_mdata) {
 	case nss_lt_name:
-		name = va_arg(ap, char *);
+		va_arg(ap, char *);
 		break;
 	case nss_lt_id:
-		uid = va_arg(ap, uid_t);
+		va_arg(ap, uid_t);
 		break;
 	case nss_lt_all:
 		break;
_at__at_ -921,7 +916,7 _at__at_
 		    errnop);
 	} while (how == nss_lt_all && !(rv & NS_TERMINATE));
 fin:
-	if (!stayopen && st->db != NULL) {
+	if (st->db != NULL && !stayopen) {
 		(void)st->db->close(st->db);
 		st->db = NULL;
 	}
_at__at_ -1876,7 +1871,6 _at__at_
 					syslog(LOG_ERR,
 					 "getpwent memory allocation failure");
 					*errnop = ENOMEM;
-					rv = NS_UNAVAIL;
 					break;
 				}
 				st->compat = COMPAT_MODE_NAME;
_at__at_ -1940,7 +1934,7 _at__at_
 			break;
 	}
 fin:
-	if (!stayopen && st->db != NULL) {
+	if (st->db != NULL && !stayopen) {
 		(void)st->db->close(st->db);
 		st->db = NULL;
 	}
Index: gen/getgrent.c
===================================================================
--- gen/getgrent.c	(revision 199242)
+++ gen/getgrent.c	(working copy)
_at__at_ -207,22 +207,19 _at__at_
 grp_marshal_func(char *buffer, size_t *buffer_size, void *retval, va_list ap,
     void *cache_mdata)
 {
-	char *name;
-	gid_t gid;
 	struct group *grp;
-	char *orig_buf;
-	size_t orig_buf_size;
 
 	struct group new_grp;
 	size_t desired_size, size, mem_size;
 	char *p, **mem;
 
+	/* advance past unused arguments */
 	switch ((enum nss_lookup_type)cache_mdata) {
 	case nss_lt_name:
-		name = va_arg(ap, char *);
+		va_arg(ap, char *);
 		break;
 	case nss_lt_id:
-		gid = va_arg(ap, gid_t);
+		va_arg(ap, gid_t);
 		break;
 	case nss_lt_all:
 		break;
_at__at_ -232,8 +229,7 _at__at_
 	}
 
 	grp = va_arg(ap, struct group *);
-	orig_buf = va_arg(ap, char *);
-	orig_buf_size = va_arg(ap, size_t);
+	va_end(ap);			/* remaining args are unused */
 
 	desired_size = _ALIGNBYTES + sizeof(struct group) + sizeof(char *);
 
_at__at_ -302,8 +298,6 _at__at_
 grp_unmarshal_func(char *buffer, size_t buffer_size, void *retval, va_list ap,
     void *cache_mdata)
 {
-	char *name;
-	gid_t gid;
 	struct group *grp;
 	char *orig_buf;
 	size_t orig_buf_size;
_at__at_ -314,10 +308,10 _at__at_
 
 	switch ((enum nss_lookup_type)cache_mdata) {
 	case nss_lt_name:
-		name = va_arg(ap, char *);
+		va_arg(ap, char *);
 		break;
 	case nss_lt_id:
-		gid = va_arg(ap, gid_t);
+		va_arg(ap, gid_t);
 		break;
 	case nss_lt_all:
 		break;
_at__at_ -1100,6 +1094,9 _at__at_
 	case nss_lt_all:
 		map = "group.byname";
 		break;
+	default:
+		*errnop = EINVAL;
+		return (NS_UNAVAIL);
 	}
 	grp     = va_arg(ap, struct group *);
 	buffer  = va_arg(ap, char *);
_at__at_ -1392,6 +1389,13 _at__at_
 			}
 			break;
 		case NS_RETURN:
+			/*
+			 * If _nsdispatch() sets *errnop to ERANGE (can it?)
+			 * we need a valid file position.  Assume it might
+			 * close st->fp, too (can it?).
+			 */
+			if (st->fp != NULL)
+				pos = ftello(st->fp);
 			goto fin;
 		default:
 			break;
_at__at_ -1450,7 +1454,7 _at__at_
 		pos = ftello(st->fp);
 	}
 fin:
-	if (!stayopen && st->fp != NULL) {
+	if (st->fp != NULL && !stayopen) {
 		fclose(st->fp);
 		st->fp = NULL;
 	}
Index: gen/getusershell.c
===================================================================
--- gen/getusershell.c	(revision 199242)
+++ gen/getusershell.c	(working copy)
_at__at_ -124,7 +124,7 _at__at_
 	if ((fp = fopen(_PATH_SHELLS, "r")) == NULL)
 		return NS_UNAVAIL;
 
-	sp = cp = line;
+	cp = line;
 	while (fgets(cp, MAXPATHLEN + 1, fp) != NULL) {
 		while (*cp != '#' && *cp != '/' && *cp != '\0')
 			cp++;
Index: gen/pw_scan.c
===================================================================
--- gen/pw_scan.c	(revision 199242)
+++ gen/pw_scan.c	(working copy)
_at__at_ -202,7 +202,7 _at__at_
 	if (p[0])
 		pw->pw_fields |= _PWF_SHELL;
 
-	if ((p = strsep(&bp, ":"))) {			/* too many */
+	if (strsep(&bp, ":") != NULL) {			/* too many */
 fmt:		
 		if (flags & _PWSCAN_WARN)
 			warnx("corrupted entry");
Index: net/sctp_sys_calls.c
===================================================================
--- net/sctp_sys_calls.c	(revision 199242)
+++ net/sctp_sys_calls.c	(working copy)
_at__at_ -242,7 +242,8 _at__at_
 	struct sockaddr *sa;
 	struct sockaddr_in *sin;
 	struct sockaddr_in6 *sin6;
-	int i, sz, argsz;
+	int i, argsz;
+	size_t sz;
 	uint16_t sport = 0;
 
 	/* validate the flags */
_at__at_ -268,7 +269,7 _at__at_
 	for (i = 0; i < addrcnt; i++) {
 		sz = sa->sa_len;
 		if (sa->sa_family == AF_INET) {
-			if (sa->sa_len != sizeof(struct sockaddr_in))
+			if (sz != sizeof(struct sockaddr_in))
 				goto out_error;
 			sin = (struct sockaddr_in *)sa;
 			if (sin->sin_port) {
_at__at_ -284,7 +285,7 _at__at_
 				}
 			}
 		} else if (sa->sa_family == AF_INET6) {
-			if (sa->sa_len != sizeof(struct sockaddr_in6))
+			if (sz != sizeof(struct sockaddr_in6))
 				goto out_error;
 			sin6 = (struct sockaddr_in6 *)sa;
 			if (sin6->sin6_port) {
_at__at_ -318,10 +319,10 _at__at_
 	for (i = 0; i < addrcnt; i++) {
 		sz = sa->sa_len;
 		if (sa->sa_family == AF_INET) {
-			if (sa->sa_len != sizeof(struct sockaddr_in))
+			if (sz != sizeof(struct sockaddr_in))
 				goto out_error;
 		} else if (sa->sa_family == AF_INET6) {
-			if (sa->sa_len != sizeof(struct sockaddr_in6))
+			if (sz != sizeof(struct sockaddr_in6))
 				goto out_error;
 		} else {
 			/* invalid address family specified */
Index: net/getservent.c
===================================================================
--- net/getservent.c	(revision 199242)
+++ net/getservent.c	(working copy)
_at__at_ -476,11 +476,11 _at__at_
 	struct nis_state *st;
 	int rv;
 
-	enum nss_lookup_type how;
 	char *name;
 	char *proto;
 	int port;
 
+	enum nss_lookup_type how;
 	struct servent *serv;
 	char *buffer;
 	size_t bufsize;
_at__at_ -491,7 +491,8 _at__at_
 
 	name = NULL;
 	proto = NULL;
-	how = (enum nss_lookup_type)mdata;
+
+	how = (intptr_t)mdata;
 	switch (how) {
 	case nss_lt_name:
 		name = va_arg(ap, char *);
Index: posix1e/acl_delete_entry.c
===================================================================
--- posix1e/acl_delete_entry.c	(revision 199242)
+++ posix1e/acl_delete_entry.c	(working copy)
_at__at_ -89,20 +89,20 _at__at_
 		return (-1);
 	}
 
-	if ((acl->ats_acl.acl_cnt < 1) ||
-	    (acl->ats_acl.acl_cnt > ACL_MAX_ENTRIES)) {
+	if ((acl_int->acl_cnt < 1) ||
+	    (acl_int->acl_cnt > ACL_MAX_ENTRIES)) {
 		errno = EINVAL;
 		return (-1);
 	}
-	for (i = 0; i < acl->ats_acl.acl_cnt;) {
-		if (_entry_matches(&(acl->ats_acl.acl_entry[i]), entry_d)) {
+	for (i = 0; i < acl_int->acl_cnt;) {
+		if (_entry_matches(&(acl_int->acl_entry[i]), entry_d)) {
 			/* ...shift the remaining entries... */
-			for (j = i; j < acl->ats_acl.acl_cnt - 1; ++j)
-				acl->ats_acl.acl_entry[j] =
-				    acl->ats_acl.acl_entry[j+1];
+			for (j = i; j < acl_int->acl_cnt - 1; ++j)
+				acl_int->acl_entry[j] =
+				    acl_int->acl_entry[j+1];
 			/* ...drop the count and zero the unused entry... */
-			acl->ats_acl.acl_cnt--;
-			bzero(&acl->ats_acl.acl_entry[j],
+			acl_int->acl_cnt--;
+			bzero(&acl_int->acl_entry[j],
 			    sizeof(struct acl_entry));
 			acl->ats_cur_entry = 0;
 			
Index: inet/inet_cidr_pton.c
===================================================================
--- inet/inet_cidr_pton.c	(revision 199242)
+++ inet/inet_cidr_pton.c	(working copy)
_at__at_ -236,7 +236,6 _at__at_
 			endp[- i] = colonp[n - i];
 			colonp[n - i] = 0;
 		}
-		tp = endp;
 	}
 
 	memcpy(dst, tmp, NS_IN6ADDRSZ);
Received on Fri Nov 13 2009 - 04:18:52 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:58 UTC