PERFORCE change 71680 for review

[John Baldwin]

On Wednesday 23 February 2005 02:10 pm, Robert Watson wrote:
> http://perforce.freebsd.org/chv.cgi?CH=71680
>
> Change 71680 by rwatson at rwatson_paprika on 2005/02/23 19:10:39
>
> 	Re-lay out mac_bsdextended sysctl to copyin, lock+dowork+unlock,
> 	copyout.
>
> 	Suggested by:	jhb
>
> Affected files ...
>
> ..
> //depot/projects/trustedbsd/mac/sys/security/mac_bsdextended/mac_bsdextende
>d.c#87 edit
>
> Differences ...
>
> ====
> //depot/projects/trustedbsd/mac/sys/security/mac_bsdextended/mac_bsdextende
>d.c#87 (text+ko) ====
>
> @@ -152,69 +152,68 @@
>          if (index > MAC_BSDEXTENDED_MAXRULES)
>  		return (ENOENT);
>
> +	ruleptr = NULL;
> +	if (req->newptr) {
> +		error = SYSCTL_IN(req, &temprule, sizeof(temprule));
> +		if (error)
> +			return (error);
> +		MALLOC(ruleptr, struct mac_bsdextended_rule *,
> +		    sizeof(*ruleptr), M_MACBSDEXTENDED, M_WAITOK | M_ZERO);
> +	}
> +
> +	mtx_lock(&mac_bsdextended_mtx);
> +
>  	if (req->oldptr) {
> -		mtx_lock(&mac_bsdextended_mtx);
>  		if (index < 0 || index > rule_slots + 1) {
> -			mtx_unlock(&mac_bsdextended_mtx);
> -			return (ENOENT);
> +			error = ENOENT;
> +			goto out;
>  		}
> -
>  		if (rules[index] == NULL) {
> -			mtx_unlock(&mac_bsdextended_mtx);
> -			return (ENOENT);
> +			error = ENOENT;
> +			goto out;
>  		}
> -
>  		temprule = *rules[index];
> -		mtx_unlock(&mac_bsdextended_mtx);
> -
> -		error = SYSCTL_OUT(req, &temprule, sizeof(temprule));
> -
> -		if (error)
> -			return (error);
>  	}
>
> -	if (req->newptr) {
> -		if (req->newlen == 0) {
> -			/* printf("deletion\n"); */
> -			mtx_lock(&mac_bsdextended_mtx);
> -			ruleptr = rules[index];
> -			if (ruleptr == NULL) {
> -				mtx_unlock(&mac_bsdextended_mtx);
> -				return (ENOENT);
> -			}
> -			rule_count--;
> -			rules[index] = NULL;
> -			mtx_unlock(&mac_bsdextended_mtx);
> -			FREE(ruleptr, M_MACBSDEXTENDED);
> -			return(0);
> +	if (req->newptr && req->newlen == 0) {
> +		/* printf("deletion\n"); */
> +		FREE(ruleptr, M_MACBSDEXTENDED);
> +		ruleptr = rules[index];
> +		if (ruleptr == NULL) {
> +			error = ENOENT;
> +			goto out;
>  		}
> -		error = SYSCTL_IN(req, &temprule, sizeof(temprule));
> -		if (error)
> -			return (error);
> -
> +		rule_count--;
> +		rules[index] = NULL;
> +	} else if (req->newptr) {
>  		error = mac_bsdextended_rule_valid(&temprule);
>  		if (error)
> -			return (error);
> +			goto out;
>
> -		MALLOC(ruleptr, struct mac_bsdextended_rule *,
> -		    sizeof(*ruleptr), M_MACBSDEXTENDED, M_WAITOK | M_ZERO);
> -		mtx_lock(&mac_bsdextended_mtx);
>  		if (rules[index] == NULL) {
>  			/* printf("addition\n"); */
>  			*ruleptr = temprule;
>  			rules[index] = ruleptr;
> +			ruleptr = NULL;
>  			if (index + 1 > rule_slots)
>  				rule_slots = index + 1;
>  			rule_count++;
> -			mtx_unlock(&mac_bsdextended_mtx);
>  		} else {
>  			/* printf("replacement\n"); */
>  			*rules[index] = temprule;
> -			mtx_unlock(&mac_bsdextended_mtx);
> -			FREE(ruleptr, M_MACBSDEXTENDED);
>  		}
>  	}
>
> +out:
> +	mtx_unlock(&mac_bsdextended_mtx);
> +	if (ruleptr != NULL)
> +		FREE(ruleptr, M_MACBSDEXTENDED);
> +	if (req->oldptr && error == 0) {
> +		error = SYSCTL_OUT(req, &temprule, sizeof(temprule));
> +		if (error)
> +			return (error);
> +	}
> +
>  	return (0);
>  }

You can simplify this last bit by making the function 'return (error);' and 
dropping the whole 'if (error) return (error);' after the SYSCTL_OUT(). :)  
Purely optional of course.

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org
To Unsubscribe: send mail to majordomo at trustedbsd.org
with "unsubscribe trustedbsd-cvs" in the body of the message