On Mon, Dec 19, 2016 at 6:52 PM, Jean-Philippe André <j...@videolan.org> wrote:
> On 20 December 2016 at 10:43, Cedric BAIL <cedric.b...@free.fr> wrote:
>> On Dec 19, 2016 16:49, "Jean-Philippe André" <j...@videolan.org> wrote:
>> On 20 December 2016 at 09:40, Cedric BAIL <cedric.b...@free.fr> wrote:
>> > cedric pushed a commit to branch master.
>> >
>> > http://git.enlightenment.org/core/efl.git/commit/?id=
>> > 7bb229d4be23ffcc4947b453880a5c0f9f7a12c6
>> >
>> > commit 7bb229d4be23ffcc4947b453880a5c0f9f7a12c6
>> > Author: Cedric BAIL <ced...@osg.samsung.com>
>> > Date:   Mon Dec 19 12:03:49 2016 -0800
>> >
>> >     eina: add general purpose function to compar float and double.
>> > ---
>> >  src/lib/eina/eina_util.h | 20 ++++++++++++++++++++
>> >  1 file changed, 20 insertions(+)
>> >
>> > diff --git a/src/lib/eina/eina_util.h b/src/lib/eina/eina_util.h
>> > index c1ea02f..66e0d17 100644
>> > --- a/src/lib/eina/eina_util.h
>> > +++ b/src/lib/eina/eina_util.h
>> > @@ -19,6 +19,8 @@
>> >  #ifndef EINA_UTIL_H_
>> >  #define EINA_UTIL_H_
>> >
>> > +#include <float.h>
>> > +
>> >  /**
>> >   * @addtogroup Eina_Tools_Group Tools
>> >   *
>> > @@ -48,6 +50,24 @@ EAPI const char *eina_environment_home_get(void);
>> >  EAPI const char *eina_environment_tmp_get(void);
>> >
>> >  /**
>> > + * @brief Safe comparison of float
>> > + * @param a First member to compar
>> > + * @param b Second member to compar
>> > + *
>> > + * @return @c true if two floats match
>> > + */
>> > +#define EINA_FLT_CMP(a, b) (fabsf((float)a - (float)b) <= FLT_EPSILON)
>> > +
>> > +/**
>> > + * @brief Safe comparison of double
>> > + * @param a First member to compar
>> > + * @param b Second member to compar
>> > + *
>> > + * @return @c true if two double match
>> > + */
>> > +#define EINA_DBL_CMP(a, b) (fabs((double)a - (double)b) <= DBL_EPSILON)
>> >
>>
>> Besides the missing @since tag, I think the names are poorly chosen. I
>> would expect a "cmp" function to return -1, 0 or 1 like strcmp().
>> How about renaming to _EQ?
>>
>> Sounds indeed like a better idea. Will update tomorrow with it.
>>
>> Also, do we really need/want the cast?
>>
>> I did think about it, and I think we need, because quite a few time we do
>> compare with integer value and that may not give the intended result. I may
>> be wrong there, but that was the logic.
>
> Yep but then I thought maybe it's better to have the cast be explicit where
> the macro is called, rather than implicitely inside the macro.
> Anyway not a big deal.

I did think about it a little bit more and there is so many place in
our code without a cast where there should be a cast, that I think it
is safer and better to actually have it in the macro. My current plan
is to rename the macro once everyone is ok with the name and do just
one patch for the rename as it won't be a behavior change.
-- 
Cedric BAIL

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/intel
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to