CFR: Exceedingly minor fixes to libc

[Jilles Tjoelker]

On Fri, Nov 13, 2009 at 12:18:49AM -0500, Garrett Wollman wrote:
> 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.

This looks mostly ok, with a few exceptions.

> [...]
> Index: gen/getpwent.c
> ===================================================================
> --- gen/getpwent.c	(revision 199242)
> +++ gen/getpwent.c	(working copy)
> @@ -257,22 +257,19 @@
> [...]
> @@ -1876,7 +1871,6 @@
>  					syslog(LOG_ERR,
>  					 "getpwent memory allocation failure");
>  					*errnop = ENOMEM;
> -					rv = NS_UNAVAIL;
>  					break;
>  				}
>  				st->compat = COMPAT_MODE_NAME;

I think this change hides the wrongness in the handling of malloc errors
while leaving it as broken as it is.

> [...]
> 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 *);

In what way is this an improvement?

> 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);
>  	}

If you are changing this code anyway, consider fixing a style bug
(extraneous parentheses) here.

> -	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;
>  			

-- 
Jilles Tjoelker