Re: [libvirt] [PATCH 53/75] qemu: Use g_strdup_printf() instead of virAsprintf()

On 10/22/19 10:57 AM, Michal Privoznik wrote: Extra line break after g_strdup_printf() here ... and here. [...] > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -262,7 +262,7 @@ qemuVirCommandGetFDSet(virCommandPtr cmd, int fd) > return NULL; > } > - virAsprintf(&result, "set=%d,fd=%d", idx, fd); > + result = g_strdup_printf("set=%d,fd=%d", idx, fd); > return result; > } [...] > @@ -3697,14 +3697,14 @@ qemuBuildLegacyNicStr(virDomainNetDefPtr net) > char macaddr[VIR_MAC_STRING_BUFLEN]; > const char *netmodel = virDomainNetGetModelString(net); > - virAsprintf(&str, > - "nic,macaddr=%s,netdev=host%s%s%s%s%s", > - virMacAddrFormat(&net->mac, macaddr), > - net->info.alias, > - netmodel ? ",model=" : "", > - NULLSTR_EMPTY(netmodel), > - (net->info.alias ? ",id=" : ""), > - NULLSTR_EMPTY(net->info.alias)); > + str = g_strdup_printf( > + "nic,macaddr=%s,netdev=host%s%s%s%s%s", > + virMacAddrFormat(&net->mac, macaddr), > + net->info.alias, > + netmodel ? ",model=" : "", > + NULLSTR_EMPTY(netmodel), > + (net->info.alias ? ",id=" : ""), > + NULLSTR_EMPTY(net->info.alias)); Extra line break after the '('. > return str; > } > @@ -4698,7 +4698,7 @@ qemuBuildVhostUserChardevStr(const char *alias, > return NULL; > } > - virAsprintf(&chardev, "socket,id=chr-vu-%s,fd=%d", alias, *fd); > + chardev = g_strdup_printf("socket,id=chr-vu-%s,fd=%d", alias, *fd); > virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); > *fd = -1; > @@ -5543,7 +5543,7 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, > if (virSCSIVHostOpenVhostSCSI(&vhostfd) < 0) > return -1; > - virAsprintf(&vhostfdName, "%d", vhostfd); > + vhostfdName = g_strdup_printf("%d", vhostfd); > virCommandPassFD(cmd, vhostfd, > VIR_COMMAND_PASS_FD_CLOSE_PARENT); > @@ -5777,7 +5777,7 @@ qemuBuildRNGBackendProps(virDomainRNGDefPtr rng, > g_autofree char *objAlias = NULL; > g_autofree char *charBackendAlias = NULL; > - virAsprintf(&objAlias, "obj%s", rng->info.alias); > + objAlias = g_strdup_printf("obj%s", rng->info.alias); > switch ((virDomainRNGBackend) rng->backend) { > case VIR_DOMAIN_RNG_BACKEND_RANDOM: > @@ -8566,7 +8566,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, > int slirpfd = qemuSlirpGetFD(slirp); > virCommandPassFD(cmd, slirpfd, > VIR_COMMAND_PASS_FD_CLOSE_PARENT); > - virAsprintf(&slirpfdName, "%d", slirpfd); > + slirpfdName = g_strdup_printf("%d", slirpfd); > } > @@ -8574,14 +8574,14 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, > if (qemuSecuritySetTapFDLabel(driver->securityManager, > def, tapfd[i]) < 0) > goto cleanup; > - virAsprintf(&tapfdName[i], "%d", tapfd[i]); > + tapfdName[i] = g_strdup_printf("%d", tapfd[i]); > virCommandPassFD(cmd, tapfd[i], > VIR_COMMAND_PASS_FD_CLOSE_PARENT); > tapfd[i] = -1; > } > for (i = 0; i < vhostfdSize; i++) { > - virAsprintf(&vhostfdName[i], "%d", vhostfd[i]); > + vhostfdName[i] = g_strdup_printf("%d", vhostfd[i]); > virCommandPassFD(cmd, vhostfd[i], > VIR_COMMAND_PASS_FD_CLOSE_PARENT); > vhostfd[i] = -1; > @@ -8957,9 +8957,9 @@ qemuBuildShmemBackendMemProps(virDomainShmemDefPtr shmem) > g_autofree char *mem_path = NULL; > virJSONValuePtr ret = NULL; > - virAsprintf(&mem_path, "/dev/shm/%s", shmem->name); > + mem_path = g_strdup_printf("/dev/shm/%s", shmem->name); > - virAsprintf(&mem_alias, "shmmem-%s", shmem->info.alias); > + mem_alias = g_strdup_printf("shmmem-%s", shmem->info.alias); > qemuMonitorCreateObjectProps(&ret, "memory-backend-file", mem_alias, > "s:mem-path", mem_path, > @@ -9689,8 +9689,8 @@ qemuBuildTPMBackendStr(const virDomainDef *def, > case VIR_DOMAIN_TPM_TYPE_EMULATOR: > virBufferAddLit(&buf, ",chardev=chrtpm"); > - virAsprintf(chardev, "socket,id=chrtpm,path=%s", > - tpm->data.emulator.source.data.nix.path); > + *chardev = g_strdup_printf("socket,id=chrtpm,path=%s", > + tpm->data.emulator.source.data.nix.path); Misalignment > break; > case VIR_DOMAIN_TPM_TYPE_LAST: > @@ -9775,13 +9775,13 @@ qemuBuildSEVCommandLine(virDomainObjPtr vm, virCommandPtr cmd, > virBufferAsprintf(&buf, ",policy=0x%x", sev->policy); > if (sev->dh_cert) { > - virAsprintf(&path, "%s/dh_cert.base64", priv->libDir); > + path = g_strdup_printf("%s/dh_cert.base64", priv->libDir); > virBufferAsprintf(&buf, ",dh-cert-file=%s", path); > VIR_FREE(path); > } > if (sev->session) { > - virAsprintf(&path, "%s/session.base64", priv->libDir); > + path = g_strdup_printf("%s/session.base64", priv->libDir); > virBufferAsprintf(&buf, ",session-file=%s", path); > VIR_FREE(path); > } [...] > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 25d4a0eb07..fe97e13c4f 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -2025,11 +2025,11 @@ qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver, > virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > if (!priv->libDir) > - virAsprintf(&priv->libDir, "%s/domain-%s", cfg->libDir, vm->def->name); > + priv->libDir = g_strdup_printf("%s/domain-%s", cfg->libDir, vm->def->name); > if (!priv->channelTargetDir) > - virAsprintf(&priv->channelTargetDir, "%s/domain-%s", > - cfg->channelTargetDir, vm->def->name); > + priv->channelTargetDir = g_strdup_printf("%s/domain-%s", > + cfg->channelTargetDir, vm->def->name); > virObjectUnref(cfg); > } > @@ -2048,11 +2048,11 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver, > goto cleanup; > if (!priv->libDir) > - virAsprintf(&priv->libDir, "%s/domain-%s", cfg->libDir, domname); > + priv->libDir = g_strdup_printf("%s/domain-%s", cfg->libDir, domname); > if (!priv->channelTargetDir) > - virAsprintf(&priv->channelTargetDir, "%s/domain-%s", > - cfg->channelTargetDir, domname); > + priv->channelTargetDir = g_strdup_printf("%s/domain-%s", > + cfg->channelTargetDir, domname); > ret = 0; > cleanup: > @@ -9316,7 +9316,7 @@ qemuDomainLogContextPtr qemuDomainLogContextNew(virQEMUDriverPtr driver, > ctxt->writefd = -1; > ctxt->readfd = -1; > - virAsprintf(&ctxt->path, "%s/%s.log", cfg->logDir, vm->def->name); > + ctxt->path = g_strdup_printf("%s/%s.log", cfg->logDir, vm->def->name); > if (cfg->stdioLogD) { > ctxt->manager = virLogManagerNew(virQEMUDriverIsPrivileged(driver)); > @@ -9496,7 +9496,7 @@ qemuDomainLogAppendMessage(virQEMUDriverPtr driver, > VIR_DEBUG("Append log message (vm='%s' message='%s) stdioLogD=%d", > vm->def->name, message, cfg->stdioLogD); > - virAsprintf(&path, "%s/%s.log", cfg->logDir, vm->def->name); > + path = g_strdup_printf("%s/%s.log", cfg->logDir, vm->def->name); > if (cfg->stdioLogD) { > if (!(manager = virLogManagerNew(virQEMUDriverIsPrivileged(driver)))) > @@ -9589,14 +9589,14 @@ qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, > if (newxml == NULL) > return -1; > - virAsprintf(&snapDir, "%s/%s", snapshotDir, vm->def->name); > + snapDir = g_strdup_printf("%s/%s", snapshotDir, vm->def->name); > if (virFileMakePath(snapDir) < 0) { > virReportSystemError(errno, _("cannot create snapshot directory '%s'"), > snapDir); > goto cleanup; > } > - virAsprintf(&snapFile, "%s/%s.xml", snapDir, def->parent.name); > + snapFile = g_strdup_printf("%s/%s.xml", snapDir, def->parent.name); > ret = virXMLSaveFile(snapFile, NULL, "snapshot-edit", newxml); > @@ -9729,8 +9729,8 @@ qemuDomainSnapshotDiscard(virQEMUDriverPtr driver, > } > } > - virAsprintf(&snapFile, "%s/%s/%s.xml", cfg->snapshotDir, vm->def->name, > - snap->def->name); > + snapFile = g_strdup_printf("%s/%s/%s.xml", cfg->snapshotDir, vm->def->name, > + snap->def->name); > if (snap == virDomainSnapshotGetCurrent(vm->snapshots)) { > virDomainSnapshotSetCurrent(vm->snapshots, NULL); > @@ -9818,7 +9818,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, > VIR_WARN("unable to remove all snapshots for domain %s", > vm->def->name); > } else { > - virAsprintf(&snapDir, "%s/%s", cfg->snapshotDir, vm->def->name); > + snapDir = g_strdup_printf("%s/%s", cfg->snapshotDir, vm->def->name); > if (rmdir(snapDir) < 0 && errno != ENOENT) > VIR_WARN("unable to remove snapshot directory %s", snapDir); > @@ -9828,8 +9828,8 @@ qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, > VIR_WARN("unable to remove all checkpoints for domain %s", > vm->def->name); > } else { > - virAsprintf(&chkDir, "%s/%s", cfg->checkpointDir, > - vm->def->name); > + chkDir = g_strdup_printf("%s/%s", cfg->checkpointDir, > + vm->def->name); > if (rmdir(chkDir) < 0 && errno != ENOENT) > VIR_WARN("unable to remove checkpoint directory %s", chkDir); > } > @@ -10177,7 +10177,7 @@ qemuDomainStorageAlias(const char *device, int depth) > if (!depth) > alias = g_strdup(device); > else > - virAsprintf(&alias, "%s.%d", device, depth); > + alias = g_strdup_printf("%s.%d", device, depth); > return alias; > } > @@ -11726,8 +11726,8 @@ ppc64VFIODeviceIsNV2Bridge(const char *device) > for (i = 0; i < G_N_ELEMENTS(nvlink2Files); i++) { > g_autofree char *file = NULL; > - virAsprintf(&file, "/sys/bus/pci/devices/%s/of_node/%s", > - device, nvlink2Files[i]); > + file = g_strdup_printf("/sys/bus/pci/devices/%s/of_node/%s", > + device, nvlink2Files[i]); > if (!virFileExists(file)) > return false; > @@ -12406,15 +12406,15 @@ qemuDomainPrepareChannel(virDomainChrDefPtr channel, > return 0; > if (channel->target.name) { > - virAsprintf(&channel->source->data.nix.path, "%s/%s", > - domainChannelTargetDir, channel->target.name); > + channel->source->data.nix.path = g_strdup_printf("%s/%s", > + domainChannelTargetDir, channel->target.name); Put channel->target.name in the next line to avoid a large line. Extra line break after '('. > } > return 0; > @@ -12604,9 +12604,9 @@ qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) > shmem->server.chr.data.nix.path) > return; > - virAsprintf(&shmem->server.chr.data.nix.path, > - "/var/lib/libvirt/shmem-%s-sock", > - shmem->name); > + shmem->server.chr.data.nix.path = g_strdup_printf( > + "/var/lib/libvirt/shmem-%s-sock", > + shmem->name); Extra line break after '('. > } [...] > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index 6495447188..c533819f4d 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -961,7 +961,7 @@ qemuMonitorNextCommandID(qemuMonitorPtr mon) > { > char *id; > - virAsprintf(&id, "libvirt-%d", ++mon->nextSerial); > + id = g_strdup_printf("libvirt-%d", ++mon->nextSerial); > return id; > } > @@ -2560,8 +2560,8 @@ qemuMonitorMigrateToHost(qemuMonitorPtr mon, > QEMU_CHECK_MONITOR(mon); > if (strchr(hostname, ':')) { > - virAsprintf(&uri, "%s:[%s]:%d", protocol, hostname, port); > - } else virAsprintf(&uri, "%s:%s:%d", protocol, hostname, port); > + uri = g_strdup_printf("%s:[%s]:%d", protocol, hostname, port); > + } else uri = g_strdup_printf("%s:%s:%d", protocol, hostname, port); > ret = qemuMonitorJSONMigrate(mon, flags, uri); > @@ -4246,20 +4246,20 @@ qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info) > switch (info->type) { > case QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV: > - virAsprintf(&ret, > - "hyper-v: arg1='0x%llx', arg2='0x%llx', " > - "arg3='0x%llx', arg4='0x%llx', arg5='0x%llx'", > - info->data.hyperv.arg1, info->data.hyperv.arg2, > - info->data.hyperv.arg3, info->data.hyperv.arg4, > - info->data.hyperv.arg5); > + ret = g_strdup_printf( > + "hyper-v: arg1='0x%llx', arg2='0x%llx', " > + "arg3='0x%llx', arg4='0x%llx', arg5='0x%llx'", > + info->data.hyperv.arg1, info->data.hyperv.arg2, > + info->data.hyperv.arg3, info->data.hyperv.arg4, > + info->data.hyperv.arg5); Extra line break after '('. Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt;