Re: [PATCH 7/8] dm: __LOW macro fix no. 2

[Jakob Oestergaard <jakob@unthought.net>]

On Wed, Feb 26, 2003 at 05:12:49PM +0000, Joe Thornber wrote:
> Another fix for the __LOW macro.
> 
> When dm_table and dm_target structures are initialized, the "limits" fields 
> (struct io_restrictions) are initialized to zero (e.g. in dm_table_add_target()
> in dm-table.c). However, zero is not a useable value in these fields. The
> request queue will never let an I/O through, regardless of how small it might
> be, if max_sectors is set to zero (see generic_make_request in ll_rw_blk.c).
> This change to the __LOW() macro sets these fields correctly when they are
> first initialized.  [Kevin Corry]
> 
> --- diff/drivers/md/dm-table.c	2003-02-26 16:10:02.000000000 +0000
> +++ source/drivers/md/dm-table.c	2003-02-26 16:10:19.000000000 +0000
> @@ -79,7 +79,7 @@
>  }
>  
>  #define __HIGH(l, r) if (*(l) < (r)) *(l) = (r)
> -#define __LOW(l, r) if (*(l) > (r)) *(l) = (r)
> +#define __LOW(l, r) if (*(l) == 0 || *(l) > (r)) *(l) = (r)

As someone else suggested, it would be good style to just use the
existing min() and max() macros.

Special-casing like the above is a recipe for disaster - having a
macro named "__LOW" which actually translates to  "min() unless left
argument is zero in which case we do a max() instead" is going to get
someone in trouble one day.

I'd say use "min()" and if there are places in the code where you want
min() unless that will be zero, then make that condition *explicit* at
those places in the code.

If you want a magic "usually_min_but_sometimes_max()" macro, then make
it's *name* reflect it's voodoo propeties.

Just my 0.02 Euro

-- 
................................................................
:   jakob@unthought.net   : And I see the elder races,         :
:.........................: putrid forms of man                :
:   Jakob Østergaard      : See him rise and claim the earth,  :
:        OZ9ABN           : his downfall is at hand.           :
:.........................:............{Konkhra}...............: