From owner-freebsd-hackers@FreeBSD.ORG Fri Nov 13 05:39:15 2009 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CD1B51065670 for ; Fri, 13 Nov 2009 05:39:15 +0000 (UTC) (envelope-from wollman@hergotha.csail.mit.edu) Received: from hergotha.csail.mit.edu (hergotha.csail.mit.edu [66.92.79.170]) by mx1.freebsd.org (Postfix) with ESMTP id 533888FC08 for ; Fri, 13 Nov 2009 05:39:14 +0000 (UTC) Received: from hergotha.csail.mit.edu (localhost [127.0.0.1]) by hergotha.csail.mit.edu (8.14.3/8.14.3) with ESMTP id nAD5In3g036054; Fri, 13 Nov 2009 00:18:49 -0500 (EST) (envelope-from wollman@hergotha.csail.mit.edu) Received: (from wollman@localhost) by hergotha.csail.mit.edu (8.14.3/8.14.3/Submit) id nAD5In2e036051; Fri, 13 Nov 2009 00:18:49 -0500 (EST) (envelope-from wollman) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="AykRJN6Qt4" Content-Transfer-Encoding: 7bit Message-ID: <19196.60473.337121.565916@hergotha.csail.mit.edu> Date: Fri, 13 Nov 2009 00:18:49 -0500 From: Garrett Wollman To: current@freebsd.org, hackers@freebsd.org X-Mailer: VM 7.17 under 21.4 (patch 21) "Educational Television" XEmacs Lucid X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-3.0 (hergotha.csail.mit.edu [127.0.0.1]); Fri, 13 Nov 2009 00:18:49 -0500 (EST) X-Spam-Status: No, score=-1.4 required=5.0 tests=ALL_TRUSTED autolearn=disabled version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on hergotha.csail.mit.edu X-Mailman-Approved-At: Fri, 13 Nov 2009 12:18:14 +0000 Cc: Subject: CFR: Exceedingly minor fixes to libc X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Nov 2009 05:39:16 -0000 --AykRJN6Qt4 Content-Type: text/plain; charset=us-ascii Content-Description: message body text Content-Transfer-Encoding: 7bit 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 --AykRJN6Qt4 Content-Type: text/plain Content-Description: patch for libc Content-Disposition: inline; filename="libc.diff.head" Content-Transfer-Encoding: 7bit Index: stdio/fvwrite.c =================================================================== --- stdio/fvwrite.c (revision 199242) +++ stdio/fvwrite.c (working copy) @@ -60,7 +60,7 @@ 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) @@ -293,7 +293,7 @@ * 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) @@ -64,7 +64,6 @@ intmax_t t, tx; int i, prec, nsec; - prec = 0; if (pi->is_long) { tv = *((struct timeval **)arg[0]); t = tv->tv_sec; @@ -78,6 +77,8 @@ } 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) @@ -89,7 +89,7 @@ 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) @@ -412,13 +412,13 @@ * 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) @@ -176,9 +176,8 @@ 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) @@ -92,13 +92,13 @@ 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); @@ -110,6 +110,9 @@ 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) @@ -141,7 +141,7 @@ 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) @@ -698,7 +698,7 @@ 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) @@ -89,7 +89,6 @@ 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); @@ -123,8 +122,7 @@ 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); @@ -162,7 +160,7 @@ 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); @@ -190,7 +188,7 @@ 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) @@ -302,7 +302,7 @@ 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) @@ -241,7 +241,7 @@ 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) @@ -86,7 +86,7 @@ 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" @@ -147,6 +147,7 @@ } #define ADDCMD(a, b, c, d) \ + do { \ if (set >= endset) { \ BITCMD *newset; \ setlen += SET_LEN_INCR; \ @@ -161,7 +162,8 @@ 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) @@ -173,11 +175,12 @@ 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. @@ -299,16 +302,10 @@ * 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) @@ -320,7 +320,7 @@ 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) @@ -647,7 +647,7 @@ 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; @@ -658,7 +658,7 @@ (void)cgetclose(); return (-1); } - for(;;) { + for (;;) { if (toprec && !gottoprec) { gottoprec = 1; line = toprec; @@ -709,7 +709,6 @@ /* * 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) @@ -257,22 +257,19 @@ 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; @@ -282,8 +279,7 @@ } 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; @@ -361,8 +357,6 @@ 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; @@ -370,12 +364,13 @@ 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; @@ -921,7 +916,7 @@ 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; } @@ -1876,7 +1871,6 @@ syslog(LOG_ERR, "getpwent memory allocation failure"); *errnop = ENOMEM; - rv = NS_UNAVAIL; break; } st->compat = COMPAT_MODE_NAME; @@ -1940,7 +1934,7 @@ 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) @@ -207,22 +207,19 @@ 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; @@ -232,8 +229,7 @@ } 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 *); @@ -302,8 +298,6 @@ 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; @@ -314,10 +308,10 @@ 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; @@ -1100,6 +1094,9 @@ 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 *); @@ -1392,6 +1389,13 @@ } 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; @@ -1450,7 +1454,7 @@ 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) @@ -124,7 +124,7 @@ 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) @@ -202,7 +202,7 @@ 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) @@ -242,7 +242,8 @@ 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 */ @@ -268,7 +269,7 @@ 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) { @@ -284,7 +285,7 @@ } } } 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) { @@ -318,10 +319,10 @@ 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) @@ -476,11 +476,11 @@ 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; @@ -491,7 +491,8 @@ 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) @@ -89,20 +89,20 @@ 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) @@ -236,7 +236,6 @@ endp[- i] = colonp[n - i]; colonp[n - i] = 0; } - tp = endp; } memcpy(dst, tmp, NS_IN6ADDRSZ); --AykRJN6Qt4--