Re: [Xen-devel] [PATCH 24/28] libxl: ocaml: add NIC helper functions

[Rob Hoes <Rob.Hoes@xxxxxxxxxx>]

[...]
> > +   if (!c_list && nb > 0)
> 
> I don't think && nb > 0 can ever occur, the error handling in
> libxl_device_nic_lsit does:
>         out_err:
>             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Unable to list nics");
>             while (*num) {
>                 (*num)--;
>                 libxl_device_nic_dispose(&nics[*num]);
>             }
>             free(nics);
>             return NULL;
> i.e. it counts *num back down to zero. I'd say you shouldn't/mustn't make
> any assumptions about nb if the function call failed.

Right, I'll fix that. I'm not sure why I had it that way.

> > +           failwith_xl(ERROR_FAIL, "nic_list");
> > +
> > +   list = temp = Val_emptylist;
> > +   for (i = 0; i < nb; i++) {
> > +           list = caml_alloc_small(2, Tag_cons);
> > +           Field(list, 0) = Val_int(0);
> > +           Field(list, 1) = temp;
> > +           temp = list;
> 
> This reverses the list, if you care. I don't suppose you do and libxl probably
> doesn't actually guarantee anything abort the order.

Yeah, I don't think the order really matters.

Cheers,
Rob

> I wouldn't have noticed except I saw you doing the counting backwards in an
> earlier patch and it took me a second to work out why...
> 
> > +           Store_field(list, 0, Val_device_nic(&c_list[i]));
> > +           libxl_device_nic_dispose(&c_list[i]);
> > +   }
> > +   free(c_list);
> > +
> > +   CAMLreturn(list);
> > +}
> > +
> >  value stub_xl_physinfo_get(value ctx)  {
> >     CAMLparam1(ctx);
> 

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