On 05/03/2013 08:53 AM, Michal Privoznik wrote:
> ---
> src/libxl/libxl_conf.c | 86 ++++++++++++++----------------------------------
> src/libxl/libxl_driver.c | 14 +++-----
> 2 files changed, 29 insertions(+), 71 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> @@ -418,37 +414,26 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
> b_info->shadow_memkb = 4 * (256 * libxl_bitmap_count_set(&b_info->avail_vcpus) +
> 2 * (b_info->max_memkb / 1024));
> } else {
> - if (def->os.bootloader) {
> - if ((b_info->u.pv.bootloader = strdup(def->os.bootloader)) == NULL) {
> - virReportOOMError();
> - goto error;
> - }
> - }
> + if (def->os.bootloader &&
> + VIR_STRDUP(b_info->u.pv.bootloader, def->os.bootloader) < 0)
You can pass NULL source arg now :)
> + goto error;
> if (def->os.bootloaderArgs) {
> if (!(b_info->u.pv.bootloader_args =
> virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
> goto error;
> }
> - if (def->os.cmdline) {
> - if ((b_info->u.pv.cmdline = strdup(def->os.cmdline)) == NULL) {
> - virReportOOMError();
> - goto error;
> - }
> - }
> + if (def->os.cmdline &&
> + VIR_STRDUP(b_info->u.pv.cmdline, def->os.cmdline) < 0)
> + goto error;
and again...
> if (def->os.kernel) {
> - /* libxl_init_build_info() sets kernel.path = strdup("hvmloader") */
> + /* libxl_init_build_info() sets VIR_STRDUP(kernel.path, "hvmloader") */
> VIR_FREE(b_info->u.pv.kernel);
> - if ((b_info->u.pv.kernel = strdup(def->os.kernel)) == NULL) {
> - virReportOOMError();
> + if (VIR_STRDUP(b_info->u.pv.kernel, def->os.kernel) < 0)
> goto error;
[Not here, of course]
> - }
> - }
> - if (def->os.initrd) {
> - if ((b_info->u.pv.ramdisk = strdup(def->os.initrd)) == NULL) {
> - virReportOOMError();
> - goto error;
> - }
> }
> + if (def->os.initrd &&
> + VIR_STRDUP(b_info->u.pv.ramdisk, def->os.initrd) < 0)
> + goto error;
> }
...and a third time.
> @@ -461,15 +446,11 @@ error:
> int
> libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk)
> {
> - if (l_disk->src && (x_disk->pdev_path = strdup(l_disk->src)) == NULL) {
> - virReportOOMError();
> + if (l_disk->src && VIR_STRDUP(x_disk->pdev_path, l_disk->src) < 0)
> return -1;
> - }
>
> - if (l_disk->dst && (x_disk->vdev = strdup(l_disk->dst)) == NULL) {
> - virReportOOMError();
> + if (l_disk->dst && VIR_STRDUP(x_disk->vdev, l_disk->dst) < 0)
> return -1;
> - }
These can be simplified as well.
> @@ -575,31 +556,22 @@ libxlMakeNic(virDomainNetDefPtr l_nic, libxl_device_nic *x_nic)
> virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
>
> if (l_nic->model && !STREQ(l_nic->model, "netfront")) {
> - if ((x_nic->model = strdup(l_nic->model)) == NULL) {
> - virReportOOMError();
> + if (VIR_STRDUP(x_nic->model, l_nic->model) < 0)
> return -1;
> - }
> x_nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
> } else {
> x_nic->nictype = LIBXL_NIC_TYPE_VIF;
> }
>
> - if (l_nic->ifname && (x_nic->ifname = strdup(l_nic->ifname)) == NULL) {
> - virReportOOMError();
> + if (l_nic->ifname && VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
> return -1;
> - }
And this one.
>
> if (l_nic->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> if (l_nic->data.bridge.brname &&
> - (x_nic->bridge = strdup(l_nic->data.bridge.brname)) == NULL) {
> - virReportOOMError();
> + VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
> return -1;
> - }
> - if (l_nic->script &&
> - (x_nic->script = strdup(l_nic->script)) == NULL) {
> - virReportOOMError();
> + if (l_nic->script && VIR_STRDUP(x_nic->script, l_nic->script) < 0)
> return -1;
And these two.
> @@ -656,16 +628,11 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
> case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> libxl_defbool_set(&x_vfb->sdl.enable, 1);
> if (l_vfb->data.sdl.display &&
> - (x_vfb->sdl.display = strdup(l_vfb->data.sdl.display)) == NULL) {
> - virReportOOMError();
> + VIR_STRDUP(x_vfb->sdl.display, l_vfb->data.sdl.display) < 0)
> return -1;
> - }
> if (l_vfb->data.sdl.xauth &&
> - (x_vfb->sdl.xauthority =
> - strdup(l_vfb->data.sdl.xauth)) == NULL) {
> - virReportOOMError();
> + VIR_STRDUP(x_vfb->sdl.xauthority, l_vfb->data.sdl.xauth) < 0)
> return -1;
> - }
And some more.
> @@ -686,19 +653,14 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
>
> listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
> if (listenAddr) {
> - /* libxl_device_vfb_init() does strdup("127.0.0.1") */
> + /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */
> VIR_FREE(x_vfb->vnc.listen);
> - if ((x_vfb->vnc.listen = strdup(listenAddr)) == NULL) {
> - virReportOOMError();
> + if (VIR_STRDUP(x_vfb->vnc.listen, listenAddr) < 0)
> return -1;
> - }
[this one can't be simplified]
> }
> if (l_vfb->data.vnc.keymap &&
> - (x_vfb->keymap =
> - strdup(l_vfb->data.vnc.keymap)) == NULL) {
> - virReportOOMError();
> + VIR_STRDUP(x_vfb->keymap, l_vfb->data.vnc.keymap) < 0)
> return -1;
> - }
Another case for simplification.
> +++ b/src/libxl/libxl_driver.c
> @@ -3879,18 +3878,18 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams)
> *nparams = 0;
> switch (sched_id) {
> case LIBXL_SCHEDULER_SEDF:
> - ret = strdup("sedf");
> + ignore_value(VIR_STRDUP(ret, "sedf"));
> break;
> case LIBXL_SCHEDULER_CREDIT:
> - ret = strdup("credit");
> + ignore_value(VIR_STRDUP(ret, "credit"));
> if (nparams)
> *nparams = XEN_SCHED_CREDIT_NPARAM;
> break;
> case LIBXL_SCHEDULER_CREDIT2:
> - ret = strdup("credit2");
> + ignore_value(VIR_STRDUP(ret, "credit2"));
> break;
> case LIBXL_SCHEDULER_ARINC653:
> - ret = strdup("arinc653");
> + ignore_value(VIR_STRDUP(ret, "arinc653"));
> break;
This is correct, but I wonder if looks any better with fewer
ignore_value calls as follows:
const char *name;
switch (sched_id) {
case LIBXL_SCHEDULER_SEDF:
name = "sedf";
break;
case LIBXL_SCHEDULER_CREDIT:
name = "credit";
if (nparams)
*nparams = XEN_SCHED_CREDIT_NPARAM;
break;
...
}
ignore_value(VIR_STRDUP(ret, name));
ACK. I'm okay whether you make the simplifications possible for a NULL
source argument now or as a followup patch, and didn't spot anything
wrong with this patch as-is.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list