[prev in list] [next in list] [prev in thread] [next in thread] 

List:       enlightenment-devel
Subject:    Re: [E-devel] [EGIT] [core/efl] master 02/04: cmdline iface - fix to consume input array AND strings
From:       Carsten Haitzler (The Rasterman) <raster () rasterman ! com>
Date:       2019-02-15 12:02:01
Message-ID: 20190215120201.473e6a719662bea05b35f8a4 () rasterman ! com
[Download RAW message or body]

On Fri, 15 Feb 2019 12:05:22 +0100 Marcel Hollerbach <mail@bu5hm4n.de> said:

> Thank you for taking care of this. However:
> 
> - you broke the C# bindings. (efl_csharp_application.cs:108)
> - You forgot to annotate that the array has the ownership of the array
> (array<const(stringshare)> @owned; != (array<const(stringshare) @owned>
> @owned;

shouldn't it be:

array<const(stringshare) @owned> @owned

?

> Greetings,
>    bu5hm4n
> 
> On 2/15/19 11:27 AM, Carsten Haitzler wrote:
> > raster pushed a commit to branch master.
> > 
> > http://git.enlightenment.org/core/efl.git/commit/?id=8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> > 
> > commit 8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> > Author: Carsten Haitzler (Rasterman) <raster@rasterman.com>
> > Date:   Thu Feb 14 11:28:23 2019 +0000
> > 
> >     cmdline iface - fix to consume input array AND strings totally
> >     
> >     strings often enough are generated e.g. via "%s/%s" or "%i" or similar
> >     etc. ... i have poitned to examples, so move to make all strings
> >     consistently stringshared, fix a bug added to the efl thread code
> >     where it accessed and freed array even tho array was consumed (but not
> >     strings) in the set, and the code used free to consume not
> >     stringshare_del. fix other code and tests to match
> >     
> >     EXCTLY the kind of bugs and mistakes with this kind of design that i
> >     said would happen more often just happened...
> > ---
> >  src/lib/ecore/efl_core_command_line.c  |  3 +++
> >  src/lib/ecore/efl_core_command_line.eo |  2 +-
> >  src/lib/ecore/efl_loop.c               |  5 ++++-
> >  src/lib/ecore/efl_thread.c             |  4 +---
> >  src/tests/ecore/efl_app_test_cml.c     | 14 +++++++-------
> >  5 files changed, 16 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/lib/ecore/efl_core_command_line.c
> > b/src/lib/ecore/efl_core_command_line.c index 24cab90b0e..bd6d21f2d4 100644
> > --- a/src/lib/ecore/efl_core_command_line.c
> > +++ b/src/lib/ecore/efl_core_command_line.c
> > @@ -223,6 +223,8 @@ _efl_core_command_line_command_array_set(Eo *obj
> > EINA_UNUSED, Efl_Core_Command_L
> > eina_stringshare_del(eina_array_pop(pd->command));
> > eina_array_free(pd->command); pd->command = NULL;
> > +             for (;i < (array ? eina_array_count(array) : 0); ++i)
> > +               eina_stringshare_del(content);
> >               eina_array_free(array);
> >               return EINA_FALSE;
> >            }
> > @@ -236,6 +238,7 @@ _efl_core_command_line_command_array_set(Eo *obj
> > EINA_UNUSED, Efl_Core_Command_L _remove_invalid_chars(param);
> >          eina_array_push(pd->command, eina_stringshare_add(param));
> >          free(param);
> > +        eina_stringshare_del(content);
> >       }
> >     pd->string_command = eina_strbuf_release(command);
> >     pd->filled = EINA_TRUE;
> > diff --git a/src/lib/ecore/efl_core_command_line.eo
> > b/src/lib/ecore/efl_core_command_line.eo index 436720d9bd..25b7c88b6e 100644
> > --- a/src/lib/ecore/efl_core_command_line.eo
> > +++ b/src/lib/ecore/efl_core_command_line.eo
> > @@ -60,7 +60,7 @@ mixin @beta Efl.Core.Command_Line {
> >          return : bool; [[On success $true, $false otherwise]]
> >        }
> >        values {
> > -        array : array<string> @owned; [[An array where every array field
> > is an argument]]
> > +        array : array<const(stringshare)> @owned; [[An array where every
> > array field is an argument]] }
> >      }
> >      @property command_string {
> > diff --git a/src/lib/ecore/efl_loop.c b/src/lib/ecore/efl_loop.c
> > index 342a6f7725..1096c62bdb 100644
> > --- a/src/lib/ecore/efl_loop.c
> > +++ b/src/lib/ecore/efl_loop.c
> > @@ -390,8 +390,11 @@ ecore_loop_arguments_send(int argc, const char **argv)
> >     cml = eina_array_new(argc);
> >     for (i = 0; i < argc; i++)
> >       {
> > -        Eina_Stringshare *arg = eina_stringshare_add(argv[i]);
> > +        Eina_Stringshare *arg;
> > +
> > +        arg = eina_stringshare_add(argv[i]);
> >          eina_array_push(arga, arg);
> > +        arg = eina_stringshare_add(argv[i]);
> >          eina_array_push(cml, arg);
> >       }
> >  
> > diff --git a/src/lib/ecore/efl_thread.c b/src/lib/ecore/efl_thread.c
> > index a324af4f58..421c92dba7 100644
> > --- a/src/lib/ecore/efl_thread.c
> > +++ b/src/lib/ecore/efl_thread.c
> > @@ -277,11 +277,9 @@ _efl_thread_main(void *data, Eina_Thread t)
> >                                            it->func, it->user_data);
> >       }
> >     efl_core_command_line_command_array_set(obj, thdat->argv);
> > +   thdat->argv = NULL;
> >     efl_future_then(obj, efl_loop_job(obj),
> >                     .success = _efl_loop_arguments_send);
> > -
> > -   while (thdat->argv && eina_array_count(thdat->argv))
> > free(eina_array_pop(thdat->argv));
> > -   eina_array_free(thdat->argv);
> >     free(thdat->event_cb);
> >     thdat->event_cb = NULL;
> >  
> > diff --git a/src/tests/ecore/efl_app_test_cml.c
> > b/src/tests/ecore/efl_app_test_cml.c index 1b7cebf552..33024dabb8 100644
> > --- a/src/tests/ecore/efl_app_test_cml.c
> > +++ b/src/tests/ecore/efl_app_test_cml.c
> > @@ -23,13 +23,13 @@ _construct_array(void)
> >  {
> >     Eina_Array *array = eina_array_new(16);
> >  
> > -   eina_array_push(array, "/bin/sh");
> > -   eina_array_push(array, "-C");
> > -   eina_array_push(array, "foo");
> > -   eina_array_push(array, "--test");
> > -   eina_array_push(array, "--option=done");
> > -   eina_array_push(array, "--");
> > -   eina_array_push(array, "asdf --test");
> > +   eina_array_push(array, eina_stringshare_add("/bin/sh"));
> > +   eina_array_push(array, eina_stringshare_add("-C"));
> > +   eina_array_push(array, eina_stringshare_add("foo"));
> > +   eina_array_push(array, eina_stringshare_add("--test"));
> > +   eina_array_push(array, eina_stringshare_add("--option=done"));
> > +   eina_array_push(array, eina_stringshare_add("--"));
> > +   eina_array_push(array, eina_stringshare_add("asdf --test"));
> >     return array;
> >  }
> >  
> > 


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
Carsten Haitzler - raster@rasterman.com



_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
[prev in list] [next in list] [prev in thread] [next in thread]