checked it again to help herdsman, noticed the following issues, take
a look so it's avoided in future commits:

On Tue, Sep 12, 2017 at 9:05 PM, Vincent Torri <[email protected]> wrote:

> +static Eina_Bool
> +_eina_value_type_rectangle_convert_to(const Eina_Value_Type *type 
> EINA_UNUSED, const Eina_Value_Type *convert, const void *type_mem, void 
> *convert_mem)
> +{
> +   const Eina_Rectangle *tr = type_mem;
> +   Eina_Bool ret = EINA_FALSE;
> +
> +   if ((convert == EINA_VALUE_TYPE_STRING) ||
> +       (convert == EINA_VALUE_TYPE_STRINGSHARE))
> +     {
> +        Eina_Strbuf *buf;
> +        const char *str;
> +
> +        buf = eina_strbuf_new();
> +        eina_strbuf_append_printf(buf, "[ %i, %i, %i, %i ]",
> +                                  tr->x, tr->y, tr->w, tr->h);
> +        str = eina_strbuf_string_get(buf);
> +        ret = eina_value_type_pset(convert, convert_mem, &str);

buf is leaked. But I'd say this is of a known size, so use the stack
buf and just snprintf() to it.


> +static Eina_Bool
> +_eina_value_type_rectangle_pset(const Eina_Value_Type *type EINA_UNUSED, 
> void *mem, const void *ptr)
> +{
> +   const Eina_Rectangle *r = ptr;
> +   Eina_Rectangle *tr = mem;
> +
> +   *tr = *r;
> +   return EINA_TRUE;
> +}
> +
> +static Eina_Bool
> +_eina_value_type_rectangle_vset(const Eina_Value_Type *type, void *mem, 
> va_list args)
> +{
> +   const Eina_Rectangle *r = va_arg(args, Eina_Rectangle *);

convention is to use vset() as on-stack values, not pointers, so here you'd use:

   const Eina_Rectangle r = va_arg(args, Eina_Rectangle);

So users can do:

   eina_value_set(v, (Eina_Rectangle){.x = 0, .y = 0, .w = 100, .h = 200 });
   eina_value_set(v, r);

The pointer is only used by "pset()" (pointer set)

> +static Eina_Bool
> +_eina_value_type_rectangle_pget(const Eina_Value_Type *type EINA_UNUSED, 
> const void *mem, void *ptr)
> +{
> +   const Eina_Rectangle *tr = mem;
> +
> +   memcpy(ptr, &tr, sizeof (void*));

this is wrong (no "&tr" and sizeof(void*)), you should follow what
other types did:

   static Eina_Bool
   _eina_value_type_uint_pget(const Eina_Value_Type *type EINA_UNUSED,
const void *mem, void *ptr)
   {
      const unsigned int *tmem = mem;
      unsigned int *p = ptr;
      *p = *tmem;
      return EINA_TRUE;
   }

so your should read:

   const Eina_Rectangle *tmem = mem;
   Eina_Rectangle *p = ptr;
   *p = *tmem;


(or in not-so-clear memcpy: memcpy(p, tmem, sizeof(Eina_Rectangle));


it would be nice to add some tests to these new values, so we know the
basics work (and avoid herdsman suffering :-D)


-- 
Gustavo Sverzut Barbieri
--------------------------------------
Mobile: +55 (16) 99354-9890

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to