Re: [libvirt] [PATCH 2/3] virutil: introduce helper to set memory limits

2015-03-05 Thread Pavel Hrdina
On Thu, Mar 05, 2015 at 10:11:50AM +0100, Peter Krempa wrote:
> On Wed, Mar 04, 2015 at 17:17:06 +0100, Pavel Hrdina wrote:
> > Using this macro will ensure that the value stored in domain def will
> > never be greater than VIR_DOMAIN_MEMORY_PARAM_UNLIMITED.
> > 
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  src/util/virutil.h | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/util/virutil.h b/src/util/virutil.h
> > index d1173c1..003eee6 100644
> > --- a/src/util/virutil.h
> > +++ b/src/util/virutil.h
> > @@ -98,6 +98,10 @@ const char *virEnumToString(const char *const*types,
> >  const char *name ## TypeToString(int type); \
> >  int name ## TypeFromString(const char*type);
> >  
> > +# define VIR_SET_MEMORY_LIMIT(limit, value) \
> > +limit = (value) < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED ? (value) : \
> > +VIR_DOMAIN_MEMORY_PARAM_UNLIMITED
> 
> You should document that @value cannot have side effects as it's
> evaluated twice.
> 
> Also why don't you make an inline function that returns the truncated
> value?

I was not sure whether to use MACRO or function, but as there is a one usage
where the value would be evaluated twice, I'll change it to function.
Additionally as suggested in next patch, I'll also create a function called
virMemoryLimitIsSet because that code is repeated a lot.

Pavel

> 
> Peter


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2/3] virutil: introduce helper to set memory limits

2015-03-05 Thread Peter Krempa
On Wed, Mar 04, 2015 at 17:17:06 +0100, Pavel Hrdina wrote:
> Using this macro will ensure that the value stored in domain def will
> never be greater than VIR_DOMAIN_MEMORY_PARAM_UNLIMITED.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/util/virutil.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/util/virutil.h b/src/util/virutil.h
> index d1173c1..003eee6 100644
> --- a/src/util/virutil.h
> +++ b/src/util/virutil.h
> @@ -98,6 +98,10 @@ const char *virEnumToString(const char *const*types,
>  const char *name ## TypeToString(int type); \
>  int name ## TypeFromString(const char*type);
>  
> +# define VIR_SET_MEMORY_LIMIT(limit, value) \
> +limit = (value) < VIR_DOMAIN_MEMORY_PARAM_UNLIMITED ? (value) : \
> +VIR_DOMAIN_MEMORY_PARAM_UNLIMITED

You should document that @value cannot have side effects as it's
evaluated twice.

Also why don't you make an inline function that returns the truncated
value?

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list