| Message ID | 1413446090-30050-2-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On 10/16/2014 01:54 AM, Peter Lieven wrote: > at least in block layer we have the case of limits being defined for a > BlockDriverState. However, in this context often zero (0) has the special > meanining of undefined which means no limit. If two of those limits are > combined and the minimum is needed the minimum function should only return > zero if both parameters are zero. > > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > include/qemu/osdep.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 1565404..9a238df 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -68,6 +68,10 @@ typedef signed int int_fast16_t; > #define MAX(a, b) (((a) > (b)) ? (a) : (b)) > #endif > > +#ifndef MIN_NON_ZERO > +#define MIN_NON_ZERO(a, b) ((!!(a) && (a) < (b)) ? (a) : (b)) '(a) && expr' is already forcing the evaluation of (a) as a boolean; thus rendering the '!!(a)' conversion to boolean redundant. The patch is correct as is, so I'll leave a positive review, but if you have a reason to respin, making it two characters shorter is probably worth it. Maybe it's also worth a comment that this macro is only designed to work on unsigned values. Reviewed-by: Eric Blake <eblake@redhat.com>
On 2014-10-16 at 09:54, Peter Lieven wrote: > at least in block layer we have the case of limits being defined for a > BlockDriverState. However, in this context often zero (0) has the special > meanining of undefined which means no limit. If two of those limits are > combined and the minimum is needed the minimum function should only return > zero if both parameters are zero. > > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > include/qemu/osdep.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 1565404..9a238df 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -68,6 +68,10 @@ typedef signed int int_fast16_t; > #define MAX(a, b) (((a) > (b)) ? (a) : (b)) > #endif > > +#ifndef MIN_NON_ZERO > +#define MIN_NON_ZERO(a, b) ((!!(a) && (a) < (b)) ? (a) : (b)) I contrast to Eric I'd even like (a) != 0 instead of !!(a). I normally use just "foo" instead of "foo != 0" or "foo != NULL", but in this case I'd like (a) != 0 (or (a) > 0) because of the name "NON_ZERO". Now you have three alternatives to choose from. You're welcome. :-) Anyway, I'm fine with any, so: Reviewed-by: Max Reitz <mreitz@redhat.com> > +#endif > + > #ifndef ROUND_UP > #define ROUND_UP(n,d) (((n) + (d) - 1) & -(d)) > #endif
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 1565404..9a238df 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -68,6 +68,10 @@ typedef signed int int_fast16_t; #define MAX(a, b) (((a) > (b)) ? (a) : (b)) #endif +#ifndef MIN_NON_ZERO +#define MIN_NON_ZERO(a, b) ((!!(a) && (a) < (b)) ? (a) : (b)) +#endif + #ifndef ROUND_UP #define ROUND_UP(n,d) (((n) + (d) - 1) & -(d)) #endif
at least in block layer we have the case of limits being defined for a BlockDriverState. However, in this context often zero (0) has the special meanining of undefined which means no limit. If two of those limits are combined and the minimum is needed the minimum function should only return zero if both parameters are zero. Signed-off-by: Peter Lieven <pl@kamp.de> --- include/qemu/osdep.h | 4 ++++ 1 file changed, 4 insertions(+)