Re: [PATCH 1/2] Consolidate min_not_zero

From: Rolf Eike Beer <eike-kernel@sf-tec.de>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: jens.axboe@oracle.com, James.Bottomley@hansenpartnership.com,
	christof.schmitt@de.ibm.com, linux-scsi@vger.kernel.org
Subject: Re: [PATCH 1/2] Consolidate min_not_zero
Date: Fri, 10 Sep 2010 10:13:51 +0200	[thread overview]
Message-ID: <201009101014.00667.eike-kernel@sf-tec.de> (raw)
In-Reply-To: <1284095925-12841-2-git-send-email-martin.petersen@oracle.com>

[-- Attachment #1: Type: Text/Plain, Size: 846 bytes --]

> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -636,6 +636,12 @@ static inline void ftrace_dump(enum ftrace_dump_mode
> oops_dump_mode) { } (void) (&_max1 == &_max2);		\
>  	_max1 > _max2 ? _max1 : _max2; })
> 
> +/*
> + * Returns the minimum that is _not_ zero, unless both are zero.
> + */
> +#define min_not_zero(__x, __y) ({		\
> +	__x == 0 ? __y : ((__y == 0) ? __x : min(__x, __y)); })
> +
>  /**
>   * clamp - return a value clamped to a given range with strict
> typechecking * @val: current value

This will evaluate the value of __x and __y multiple times. Something like 
this should be better (untested):

define min_not_zero(__x, __y) ({		\
	typeof(__x) _minz1 = (__x);			\
	typeof(__y) _minz2 = (__y);			\
	_minz1 == 0 ? _minz2 : ((_minz2 == 0) ? _minz1 : min(_minz1, _minz2)); })

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]