Re: [Xen-devel] [PATCH v4 18/21] libxlu: rework internal representation of setting

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Feb 11, 2015 at 04:12:24PM +0000, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v4 18/21] libxlu: rework internal representation of 
> setting"):
> > This patches does following things:
> ...
> > +void xlu__cfg_list_append(CfgParseContext *ctx,
> > +                          XLU_ConfigValue *list,
> > +                          char *atom)
> > +{
> > +    XLU_ConfigValue **new_val = NULL;
> > +    XLU_ConfigValue *val = NULL;
> >      if (ctx->err) return;
> ...
> > -    if (set->nvalues >= set->avalues) {
> > -        int new_avalues;
> > -        char **new_values;
> > -
> > -        if (set->avalues > INT_MAX / 100) { ctx->err= ERANGE; return; }
> > -        new_avalues= set->avalues * 4;
> > -        new_values= realloc(set->values,
> > -                            sizeof(*new_values) * new_avalues);
> > -        if (!new_values) { ctx->err= errno; free(atom); return; }
> > -        set->values= new_values;
> > -        set->avalues= new_avalues;
> 
> This is a standard expanding-array pattern which arranges not to
> realloc the array each time.
> 
> > -    }
> > -    set->values[set->nvalues++]= atom;
> > +    new_val = realloc(list->u.list.values,
> > +                      sizeof(*new_val) * (list->u.list.nvalues+1));
> > +    if (!new_val) goto xe;
> 
> But you replace it here with one which has quadradic performance.
> 
> I don't know whether people are going to specify lists with hundreds
> or thousands of elements, but it doesn't seem impossible.
> 
> I think you should retain the existing avalues stuff.
> 

No problem.

> Apart from that this all looks good to me.

Thanks.

Wei.

> 
> Thanks,
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel