On Thu, 2016-02-04 at 10:23 +0100, Olaf Hering wrote: > Ian, > > in my pvscsi code I have two copies of a helper function which appends > yet another instance of something to an Array, as shown below. This is > similar to the _copy variant. Is it worth to let gentypes generate such > a helper, like libxl_device_vscsictrl_append_vscsidev()?
If something can be autogenerated without too much trouble then I see no reason not to do so. I'd go with libxl_<type>_list_append as the naming scheme, which fits in with libxl_<type>_list_free (which probably ought to be autogenerated too, but isn't). So: libxl_device_vscsidev_list_append(libxl_ctx *ctx, libxl_device_vscsidev *dev, int nr, libxl_device_vscsidev *new); (if you intend for this to be internal then s/^libxl_/&_/ and s/libxl_ctx \*ctx/libxl__gc *gc/) Oh, I see you want it to take the type containing the array, that could work to, you'd need to call it libxl_<type>_append_<field>, so libxl_device_vscsictrl_append_vscsidevs which looks a bit odd (since the field name is plural and the IDL has no way to find the singular). We could live with that, or s/append/append_to/ or make it varargs and take perhaps multiple new entries and a NULL terminator. I think the append_to variant is probably least gross. Looks like various places such as libxl__append_nic_list_of_type could make use of this helper too. As could xl_cmdimpl.c for ARRAY_EXTEND_INIT perhaps (using it everywhere isn't mandatory of course, but if you feel inclined it would be nice) > While writing this I realize that libxl__realloc will not return, so my > helper can be converted from returning int to void, and all the locals > can be removed. Indeed. > static int vscsi_append_dev(libxl__gc *gc, libxl_device_vscsictrl *ctrl, > libxl_device_vscsidev *dev) > { > int rc; > libxl_device_vscsidev *devs; > > devs = libxl__realloc(NOGC, ctrl->vscsidevs, sizeof(*dev) * > (ctrl->num_vscsidevs + 1)); > if (!devs) { > rc = ERROR_NOMEM; > goto out; > } > > ctrl->vscsidevs = devs; > libxl_device_vscsidev_init(ctrl->vscsidevs + ctrl->num_vscsidevs); > libxl_device_vscsidev_copy(CTX, ctrl->vscsidevs + ctrl->num_vscsidevs, > dev); Wei, is it necessary to init the dst before copy into it? > ctrl->num_vscsidevs++; > rc = 0; > out: > return rc; > } > > > Olaf _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel