[pulseaudio-discuss] [PATCH] dynarray: Reimplement with nicer semantics

[Tanu Kaskinen]

On Thu, 2013-06-27 at 08:37 +0200, David Henningsson wrote:
> On 06/26/2013 03:13 PM, Tanu Kaskinen wrote:
> > A dynamic array is a nice simple container, but the old interface
> > wasn't quite what I wanted it to be. I like GLib's way of providing
> > the free callback at the container creation time, because that way
> > the free callback doesn't have to be given every time something is
> > removed from the array.
> >
> > The allocation pattern was changed too: instead of increasing the
> > array size always by 25 when the array gets full, the size gets
> > doubled now. Instead of starting with zero allocated size, the initial
> > allocated size is now 25.
> >
> > The array can't store NULL pointers anymore, and pa_dynarray_get() was
> > changed so that it's forbidden to try to access elements outside the
> > valid range.
> >
> > The set of supported operations may seem a bit arbitrary. The
> > operation set is by no means complete at this point. I have included
> > only those operations that are required by the current code and some
> > unpublished code of mine.
> > ---
> >   src/pulsecore/cli-command.c |  4 +--
> >   src/pulsecore/dynarray.c    | 85 +++++++++++++++++++--------------------------
> >   src/pulsecore/dynarray.h    | 49 +++++++++++++++-----------
> >   src/pulsecore/tokenizer.c   |  8 +++--
> >   4 files changed, 72 insertions(+), 74 deletions(-)
> >
> > diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c
> > index 866cd16..6644d64 100644
> > --- a/src/pulsecore/cli-command.c
> > +++ b/src/pulsecore/cli-command.c
> > @@ -1983,7 +1983,7 @@ int pa_cli_command_execute_line_stateful(pa_core *c, const char *s, pa_strbuf *b
> >                               char **sorted_files;
> >                               struct dirent *de;
> >                               pa_bool_t failed = FALSE;
> > -                            pa_dynarray *files = pa_dynarray_new();
> > +                            pa_dynarray *files = pa_dynarray_new(NULL);
> >
> >                               while ((de = readdir(d))) {
> >                                   char *extn;
> > @@ -2003,7 +2003,7 @@ int pa_cli_command_execute_line_stateful(pa_core *c, const char *s, pa_strbuf *b
> >                               sorted_files = pa_xnew(char*, count);
> >                               for (i = 0; i < count; ++i)
> >                                   sorted_files[i] = pa_dynarray_get(files, i);
> > -                            pa_dynarray_free(files, NULL);
> > +                            pa_dynarray_free(files);
> >
> >                               for (i = 0; i < count; ++i) {
> >                                   for (unsigned j = 0; j < count; ++j) {
> > diff --git a/src/pulsecore/dynarray.c b/src/pulsecore/dynarray.c
> > index 78b2eb9..008f8d3 100644
> > --- a/src/pulsecore/dynarray.c
> > +++ b/src/pulsecore/dynarray.c
> > @@ -31,82 +31,69 @@
> >
> >   #include "dynarray.h"
> >
> > -/* If the array becomes to small, increase its size by 25 entries */
> > -#define INCREASE_BY 25
> > +#define MIN_N_ALLOCATED 25U
> >
> >   struct pa_dynarray {
> >       void **data;
> >       unsigned n_allocated, n_entries;
> > +    pa_free_cb_t free_cb;
> >   };
> >
> > -pa_dynarray* pa_dynarray_new(void) {
> > -    pa_dynarray *a;
> > +pa_dynarray* pa_dynarray_new(pa_free_cb_t free_cb) {
> > +    pa_dynarray *array;
> >
> > -    a = pa_xnew(pa_dynarray, 1);
> > -    a->data = NULL;
> > -    a->n_entries = 0;
> > -    a->n_allocated = 0;
> > +    array = pa_xnew0(pa_dynarray, 1);
> > +    array->data = pa_xnew(void *, MIN_N_ALLOCATED);
> > +    array->n_allocated = MIN_N_ALLOCATED;
> 
> With the MAX change below, we should probably default to an empty array 
> like it was done previously - it could save some memory if we have many 
> empty arrays.
> 
> If you agree, feel free to push with that change.

Yes I agree, there are no downsides to delaying the initial array
allocation. Patch pushed.

-- 
Tanu