[libvirt] [PATCH 00/10] tools: remove vshStrdup (glib chronicles)

Ján Tomko (10): tools: cmdDomblkinfoGet: reindent parameters virsh: use g_strdup in cmdDomblkinfoGet virsh: use g_strdup in virshDomainGetEditMetadata virsh: getSignalNumber: rename variables virsh: getSignalNumber: use g_autofree virsh: getSignalNumber: use g_strdup tools: vshCommandArgvGetArg: one parameter per line tools: vshCommandArgvGetArg: prefer g_strdup tools: prefer g_strdup to vshStrdup tools: delete vshStrdup tools/virsh-checkpoint.c | 4 +-- tools/virsh-domain-monitor.c | 27 ++++++++++---------- tools/virsh-domain.c | 38 ++++++++++++---------------- tools/virsh-nodedev.c | 5 ++-- tools/virsh-pool.c | 30 +++++++++++----------- tools/virsh-snapshot.c | 6 ++--- tools/virsh-volume.c | 11 ++++----- tools/virsh.c | 9 +++---- tools/virt-admin.c | 6 ++--- tools/vsh.c | 48 ++++++++++++++---------------------- tools/vsh.h | 4 --- 11 files changed, 81 insertions(+), 107 deletions(-) -- 2.21.0

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain-monitor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b2bb85cd96..cd675df07d 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -421,11 +421,11 @@ static const vshCmdOptDef opts_domblkinfo[] = { static bool cmdDomblkinfoGet(vshControl *ctl, - const virDomainBlockInfo *info, - char **cap, - char **alloc, - char **phy, - bool human) + const virDomainBlockInfo *info, + char **cap, + char **alloc, + char **phy, + bool human) { if (info->capacity == 0 && info->allocation == 0 && info->physical == 0) { *cap = vshStrdup(ctl, "-"); -- 2.21.0

Prefer GLib's g_strdup to vshStrdup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain-monitor.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index cd675df07d..739c8df381 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -420,17 +420,16 @@ static const vshCmdOptDef opts_domblkinfo[] = { }; static bool -cmdDomblkinfoGet(vshControl *ctl, - const virDomainBlockInfo *info, +cmdDomblkinfoGet(const virDomainBlockInfo *info, char **cap, char **alloc, char **phy, bool human) { if (info->capacity == 0 && info->allocation == 0 && info->physical == 0) { - *cap = vshStrdup(ctl, "-"); - *alloc = vshStrdup(ctl, "-"); - *phy = vshStrdup(ctl, "-"); + *cap = g_strdup("-"); + *alloc = g_strdup("-"); + *phy = g_strdup("-"); } else if (!human) { if (virAsprintf(cap, "%llu", info->capacity) < 0 || virAsprintf(alloc, "%llu", info->allocation) < 0 || @@ -530,7 +529,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) memset(&info, 0, sizeof(info)); } - if (!cmdDomblkinfoGet(ctl, &info, &cap, &alloc, &phy, human)) + if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human)) goto cleanup; if (vshTableRowAppend(table, target, cap, alloc, phy, NULL) < 0) goto cleanup; @@ -545,7 +544,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) if (virDomainGetBlockInfo(dom, device, &info, 0) < 0) goto cleanup; - if (!cmdDomblkinfoGet(ctl, &info, &cap, &alloc, &phy, human)) + if (!cmdDomblkinfoGet(&info, &cap, &alloc, &phy, human)) goto cleanup; vshPrint(ctl, "%-15s %s\n", _("Capacity:"), cap); vshPrint(ctl, "%-15s %s\n", _("Allocation:"), alloc); -- 2.21.0

Prefer GLib's g_strdup to vshStrdup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3ba8451470..715ad51ef5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8523,7 +8523,7 @@ static const vshCmdOptDef opts_metadata[] = { /* helper to add new metadata using the --edit option */ static char * -virshDomainGetEditMetadata(vshControl *ctl, +virshDomainGetEditMetadata(vshControl *ctl G_GNUC_UNUSED, virDomainPtr dom, const char *uri, unsigned int flags) @@ -8533,7 +8533,7 @@ virshDomainGetEditMetadata(vshControl *ctl, if (!(ret = virDomainGetMetadata(dom, VIR_DOMAIN_METADATA_ELEMENT, uri, flags))) { vshResetLibvirtError(); - ret = vshStrdup(ctl, "\n"); + ret = g_strdup("\n"); } return ret; -- 2.21.0

Use 'str' for the allocated copy of the string and 'p' for the pointer into that string. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 715ad51ef5..d74a671167 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8806,26 +8806,26 @@ static int getSignalNumber(vshControl *ctl, const char *signame) { size_t i; int signum; - char *lower = vshStrdup(ctl, signame); - char *tmp = lower; + char *str = vshStrdup(ctl, signame); + char *p = str; for (i = 0; signame[i]; i++) - lower[i] = c_tolower(signame[i]); + p[i] = c_tolower(signame[i]); - if (virStrToLong_i(lower, NULL, 10, &signum) >= 0) + if (virStrToLong_i(p, NULL, 10, &signum) >= 0) goto cleanup; - if (STRPREFIX(lower, "sig_")) - lower += 4; - else if (STRPREFIX(lower, "sig")) - lower += 3; + if (STRPREFIX(p, "sig_")) + p += 4; + else if (STRPREFIX(p, "sig")) + p += 3; - if ((signum = virDomainProcessSignalTypeFromString(lower)) >= 0) + if ((signum = virDomainProcessSignalTypeFromString(p)) >= 0) goto cleanup; signum = -1; cleanup: - VIR_FREE(tmp); + VIR_FREE(str); return signum; } -- 2.21.0

Mark the 'str' variable as g_autofree and avoid the need for a separate cleanup label. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d74a671167..44eb8a5d32 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8806,27 +8806,21 @@ static int getSignalNumber(vshControl *ctl, const char *signame) { size_t i; int signum; - char *str = vshStrdup(ctl, signame); + g_autofree char *str = vshStrdup(ctl, signame); char *p = str; for (i = 0; signame[i]; i++) p[i] = c_tolower(signame[i]); if (virStrToLong_i(p, NULL, 10, &signum) >= 0) - goto cleanup; + return signum; if (STRPREFIX(p, "sig_")) p += 4; else if (STRPREFIX(p, "sig")) p += 3; - if ((signum = virDomainProcessSignalTypeFromString(p)) >= 0) - goto cleanup; - - signum = -1; - cleanup: - VIR_FREE(str); - return signum; + return virDomainProcessSignalTypeFromString(p); } static bool -- 2.21.0

Eliminate the use of vshStrdup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 44eb8a5d32..b1618960b1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8802,11 +8802,11 @@ VIR_ENUM_IMPL(virDomainProcessSignal, "rt23", "rt24", "rt25", "rt26", "rt27", /* 55-59 */ "rt28", "rt29", "rt30", "rt31", "rt32"); /* 60-64 */ -static int getSignalNumber(vshControl *ctl, const char *signame) +static int getSignalNumber(const char *signame) { size_t i; int signum; - g_autofree char *str = vshStrdup(ctl, signame); + g_autofree char *str = g_strdup(signame); char *p = str; for (i = 0; signame[i]; i++) @@ -8840,7 +8840,7 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "signame", &signame) < 0) return false; - if ((signum = getSignalNumber(ctl, signame)) < 0) { + if ((signum = getSignalNumber(signame)) < 0) { vshError(ctl, _("malformed signal name: %s"), signame); return false; } -- 2.21.0

Split the parameters to make changes more readable. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/vsh.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/vsh.c b/tools/vsh.c index ee675a63cc..fccd4cd36d 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1644,7 +1644,9 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) */ static vshCommandToken ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) -vshCommandArgvGetArg(vshControl *ctl, vshCommandParser *parser, char **res, +vshCommandArgvGetArg(vshControl *ctl, + vshCommandParser *parser, + char **res, bool report G_GNUC_UNUSED) { if (parser->arg_pos == parser->arg_end) { -- 2.21.0

Remove the use of vshStrdup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/vsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index fccd4cd36d..9ee3f99ff3 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1644,7 +1644,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) */ static vshCommandToken ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) -vshCommandArgvGetArg(vshControl *ctl, +vshCommandArgvGetArg(vshControl *ctl G_GNUC_UNUSED, vshCommandParser *parser, char **res, bool report G_GNUC_UNUSED) @@ -1654,7 +1654,7 @@ vshCommandArgvGetArg(vshControl *ctl, return VSH_TK_END; } - *res = vshStrdup(ctl, *parser->arg_pos); + *res = g_strdup(*parser->arg_pos); parser->arg_pos++; return VSH_TK_ARG; } -- 2.21.0

Remove all the uses of vshStrdup in favor of GLib's g_strdup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-checkpoint.c | 4 ++-- tools/virsh-domain-monitor.c | 6 +++--- tools/virsh-domain.c | 4 ++-- tools/virsh-nodedev.c | 5 ++--- tools/virsh-pool.c | 30 ++++++++++++++---------------- tools/virsh-snapshot.c | 6 +++--- tools/virsh-volume.c | 11 +++++------ tools/virsh.c | 9 ++++----- tools/virt-admin.c | 6 +++--- tools/vsh.c | 30 +++++++++++++++--------------- 10 files changed, 53 insertions(+), 58 deletions(-) diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index 08f8fded86..7fd3914ef2 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -127,7 +127,7 @@ cmdCheckpointCreate(vshControl *ctl, if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) goto cleanup; if (!from) { - buffer = vshStrdup(ctl, "<domaincheckpoint/>"); + buffer = g_strdup("<domaincheckpoint/>"); } else { if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { vshSaveLibvirtError(); @@ -426,7 +426,7 @@ virshGetCheckpointParent(vshControl *ctl, parent = virDomainCheckpointGetParent(checkpoint, 0); if (parent) { /* API works, and virDomainCheckpointGetName will succeed */ - *parent_name = vshStrdup(ctl, virDomainCheckpointGetName(parent)); + *parent_name = g_strdup(virDomainCheckpointGetName(parent)); ret = 0; } else if (last_error->code == VIR_ERR_NO_DOMAIN_CHECKPOINT) { /* API works, and we found a root with no parent */ diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 739c8df381..bfff08b0f6 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -73,7 +73,7 @@ virshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool title, int errCode = virGetLastErrorCode(); if (errCode == VIR_ERR_NO_DOMAIN_METADATA) { - desc = vshStrdup(ctl, ""); + desc = g_strdup(""); vshResetLibvirtError(); return desc; } @@ -92,7 +92,7 @@ virshGetDomainDescription(vshControl *ctl, virDomainPtr dom, bool title, desc = virXPathString("string(./description[1])", ctxt); if (!desc) - desc = vshStrdup(ctl, ""); + desc = g_strdup(""); cleanup: xmlXPathFreeContext(ctxt); @@ -2419,7 +2419,7 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) ip_addr_str = virBufferContentAndReset(&buf); if (!ip_addr_str) - ip_addr_str = vshStrdup(ctl, ""); + ip_addr_str = g_strdup(""); /* Don't repeat interface name */ if (full || !j) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b1618960b1..2f3ac2d430 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5085,7 +5085,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, int rv; while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - set_field = vshStrdup(ctl, opt->data); + set_field = g_strdup(opt->data); if (!(set_val = strchr(set_field, '='))) { vshError(ctl, "%s", _("Invalid syntax for --set, " "expecting name=value")); @@ -11481,7 +11481,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) VIR_FREE(listen_addr); if (uri) { - listen_addr = vshStrdup(ctl, uri->server); + listen_addr = g_strdup(uri->server); virURIFree(uri); } } diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 455ddedc2d..cb2fc26d1a 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -477,13 +477,12 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) struct virshNodeList arrays = { names, parents }; for (i = 0; i < list->ndevices; i++) - names[i] = vshStrdup(ctl, virNodeDeviceGetName(list->devices[i])); + names[i] = g_strdup(virNodeDeviceGetName(list->devices[i])); for (i = 0; i < list->ndevices; i++) { virNodeDevicePtr dev = list->devices[i]; if (STRNEQ(names[i], "computer")) { - const char *parent = virNodeDeviceGetParent(dev); - parents[i] = parent ? vshStrdup(ctl, parent) : NULL; + parents[i] = g_strdup(virNodeDeviceGetParent(dev)); } else { parents[i] = NULL; } diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index bb25840943..bd876aefda 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1247,10 +1247,9 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) /* Retrieve the autostart status of the pool */ if (virStoragePoolGetAutostart(list->pools[i], &autostart) < 0) - poolInfoTexts[i].autostart = vshStrdup(ctl, _("no autostart")); + poolInfoTexts[i].autostart = g_strdup(_("no autostart")); else - poolInfoTexts[i].autostart = vshStrdup(ctl, autostart ? - _("yes") : _("no")); + poolInfoTexts[i].autostart = g_strdup(autostart ? _("yes") : _("no")); /* Retrieve the persistence status of the pool */ if (details) { @@ -1258,28 +1257,27 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) vshDebug(ctl, VSH_ERR_DEBUG, "Persistent flag value: %d\n", persistent); if (persistent < 0) - poolInfoTexts[i].persistent = vshStrdup(ctl, _("unknown")); + poolInfoTexts[i].persistent = g_strdup(_("unknown")); else - poolInfoTexts[i].persistent = vshStrdup(ctl, persistent ? - _("yes") : _("no")); + poolInfoTexts[i].persistent = g_strdup(persistent ? _("yes") : _("no")); } /* Collect further extended information about the pool */ if (virStoragePoolGetInfo(list->pools[i], &info) != 0) { /* Something went wrong retrieving pool info, cope with it */ vshError(ctl, "%s", _("Could not retrieve pool information")); - poolInfoTexts[i].state = vshStrdup(ctl, _("unknown")); + poolInfoTexts[i].state = g_strdup(_("unknown")); if (details) { - poolInfoTexts[i].capacity = vshStrdup(ctl, _("unknown")); - poolInfoTexts[i].allocation = vshStrdup(ctl, _("unknown")); - poolInfoTexts[i].available = vshStrdup(ctl, _("unknown")); + poolInfoTexts[i].capacity = g_strdup(_("unknown")); + poolInfoTexts[i].allocation = g_strdup(_("unknown")); + poolInfoTexts[i].available = g_strdup(_("unknown")); } } else { /* Decide which state string to display */ if (details) { const char *state = virshStoragePoolStateToString(info.state); - poolInfoTexts[i].state = vshStrdup(ctl, state); + poolInfoTexts[i].state = g_strdup(state); /* Create the pool size related strings */ if (info.state == VIR_STORAGE_POOL_RUNNING || @@ -1303,17 +1301,17 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) goto cleanup; } else { /* Capacity related information isn't available */ - poolInfoTexts[i].capacity = vshStrdup(ctl, _("-")); - poolInfoTexts[i].allocation = vshStrdup(ctl, _("-")); - poolInfoTexts[i].available = vshStrdup(ctl, _("-")); + poolInfoTexts[i].capacity = g_strdup(_("-")); + poolInfoTexts[i].allocation = g_strdup(_("-")); + poolInfoTexts[i].available = g_strdup(_("-")); } } else { /* --details option was not specified, only active/inactive * state strings are used */ if (virStoragePoolIsActive(list->pools[i])) - poolInfoTexts[i].state = vshStrdup(ctl, _("active")); + poolInfoTexts[i].state = g_strdup(_("active")); else - poolInfoTexts[i].state = vshStrdup(ctl, _("inactive")); + poolInfoTexts[i].state = g_strdup(_("inactive")); } } } diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index eae19ecd27..751186ebf8 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -197,7 +197,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) goto cleanup; if (!from) { - buffer = vshStrdup(ctl, "<domainsnapshot/>"); + buffer = g_strdup("<domainsnapshot/>"); } else { if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { vshSaveLibvirtError(); @@ -755,7 +755,7 @@ virshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot, parent = virDomainSnapshotGetParent(snapshot, 0); if (parent) { /* API works, and virDomainSnapshotGetName will succeed */ - *parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); + *parent_name = g_strdup(virDomainSnapshotGetName(parent)); ret = 0; goto cleanup; } @@ -1236,7 +1236,7 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom, count - 1, flags); if (count >= 0) { count++; - names[0] = vshStrdup(ctl, fromname); + names[0] = g_strdup(fromname); } } else { count = virDomainSnapshotListChildrenNames(from, names, diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 702d0109ad..d09d4435ad 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1413,22 +1413,21 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) /* Retrieve the volume path */ if ((volInfoTexts[i].path = virStorageVolGetPath(vol)) == NULL) { /* Something went wrong retrieving a volume path, cope with it */ - volInfoTexts[i].path = vshStrdup(ctl, _("unknown")); + volInfoTexts[i].path = g_strdup(_("unknown")); } /* If requested, retrieve volume type and sizing information */ if (details) { if (virStorageVolGetInfo(vol, &volumeInfo) != 0) { /* Something went wrong retrieving volume info, cope with it */ - volInfoTexts[i].allocation = vshStrdup(ctl, _("unknown")); - volInfoTexts[i].capacity = vshStrdup(ctl, _("unknown")); - volInfoTexts[i].type = vshStrdup(ctl, _("unknown")); + volInfoTexts[i].allocation = g_strdup(_("unknown")); + volInfoTexts[i].capacity = g_strdup(_("unknown")); + volInfoTexts[i].type = g_strdup(_("unknown")); } else { /* Convert the returned volume info into output strings */ /* Volume type */ - volInfoTexts[i].type = vshStrdup(ctl, - virshVolumeTypeToString(volumeInfo.type)); + volInfoTexts[i].type = g_strdup(virshVolumeTypeToString(volumeInfo.type)); val = vshPrettyCapacity(volumeInfo.capacity, &unit); if (virAsprintf(&volInfoTexts[i].capacity, diff --git a/tools/virsh.c b/tools/virsh.c index a3553ddd36..8c0e9d960d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -236,7 +236,7 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force) } else { if (name) { VIR_FREE(ctl->connname); - ctl->connname = vshStrdup(ctl, name); + ctl->connname = g_strdup(name); } priv->readonly = readonly; @@ -677,7 +677,7 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) switch (arg) { case 'c': VIR_FREE(ctl->connname); - ctl->connname = vshStrdup(ctl, optarg); + ctl->connname = g_strdup(optarg); break; case 'd': if (virStrToLong_i(optarg, NULL, 10, &debug) < 0) { @@ -742,7 +742,7 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) break; case 'l': vshCloseLogFile(ctl); - ctl->logfile = vshStrdup(ctl, optarg); + ctl->logfile = g_strdup(optarg); vshOpenLogFile(ctl); break; case 'q': @@ -906,8 +906,7 @@ main(int argc, char **argv) } if (!ctl->connname) - ctl->connname = vshStrdup(ctl, - getenv("VIRSH_DEFAULT_CONNECT_URI")); + ctl->connname = g_strdup(getenv("VIRSH_DEFAULT_CONNECT_URI")); if (!ctl->imode) { ret = vshCommandRun(ctl, ctl->cmd); diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 3aada5f963..f3ae011cf4 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -343,7 +343,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) if (name) { VIR_FREE(ctl->connname); - ctl->connname = vshStrdup(ctl, name); + ctl->connname = g_strdup(name); } vshAdmReconnect(ctl); @@ -1295,7 +1295,7 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv) switch (arg) { case 'c': VIR_FREE(ctl->connname); - ctl->connname = vshStrdup(ctl, optarg); + ctl->connname = g_strdup(optarg); break; case 'd': if (virStrToLong_i(optarg, NULL, 10, &debug) < 0) { @@ -1315,7 +1315,7 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv) break; case 'l': vshCloseLogFile(ctl); - ctl->logfile = vshStrdup(ctl, optarg); + ctl->logfile = g_strdup(optarg); vshOpenLogFile(ctl); break; case 'q': diff --git a/tools/vsh.c b/tools/vsh.c index 9ee3f99ff3..baba5ec314 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -176,7 +176,7 @@ int vshStringToArray(const char *str, char ***array) { - char *str_copied = vshStrdup(NULL, str); + char *str_copied = g_strdup(str); char *str_tok = NULL; char *tmp; unsigned int nstr_tokens = 0; @@ -214,10 +214,10 @@ vshStringToArray(const char *str, continue; } *tmp++ = '\0'; - arr[nstr_tokens++] = vshStrdup(NULL, str_tok); + arr[nstr_tokens++] = g_strdup(str_tok); str_tok = tmp; } - arr[nstr_tokens++] = vshStrdup(NULL, str_tok); + arr[nstr_tokens++] = g_strdup(str_tok); *array = arr; VIR_FREE(str_copied); @@ -1451,7 +1451,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) /* aliases need to be resolved to the actual commands */ if (cmd->flags & VSH_CMD_FLAG_ALIAS) { VIR_FREE(tkdata); - tkdata = vshStrdup(ctl, cmd->alias); + tkdata = g_strdup(cmd->alias); cmd = vshCmddefSearch(tkdata); } if (vshCmddefOptParse(cmd, &opts_need_arg, @@ -1472,7 +1472,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) if (optstr) { *optstr = '\0'; /* convert the '=' to '\0' */ - optstr = vshStrdup(ctl, optstr + 1); + optstr = g_strdup(optstr + 1); } /* Special case 'help' to ignore all spurious options */ if (!(opt = vshCmddefGetOption(ctl, cmd, tkdata + 2, @@ -1582,7 +1582,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) vshCommandOptFree(first); first = vshMalloc(ctl, sizeof(vshCmdOpt)); first->def = help->opts; - first->data = vshStrdup(ctl, cmd->name); + first->data = g_strdup(cmd->name); first->next = NULL; cmd = help; @@ -1686,7 +1686,7 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, bool double_quote = false; int sz = 0; char *p = parser->pos; - char *q = vshStrdup(ctl, p); + char *q = g_strdup(p); *res = q; @@ -1834,11 +1834,11 @@ vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item) break; case VIR_TYPED_PARAM_BOOLEAN: - str = vshStrdup(ctl, item->value.b ? _("yes") : _("no")); + str = g_strdup(item->value.b ? _("yes") : _("no")); break; case VIR_TYPED_PARAM_STRING: - str = vshStrdup(ctl, item->value.s); + str = g_strdup(item->value.s); break; default: @@ -2668,7 +2668,7 @@ vshReadlineCommandGenerator(const char *text) virStringListFree(ret); return NULL; } - ret[ret_size] = vshStrdup(NULL, name); + ret[ret_size] = g_strdup(name); ret_size++; /* Terminate the string list properly. */ ret[ret_size] = NULL; @@ -2820,7 +2820,7 @@ vshReadlineParse(const char *text, int state) char *ret = NULL; if (!state) { - char *buf = vshStrdup(NULL, rl_line_buffer); + char *buf = g_strdup(rl_line_buffer); vshCommandFree(partial); partial = NULL; @@ -2882,7 +2882,7 @@ vshReadlineParse(const char *text, int state) } if (list) { - ret = vshStrdup(NULL, list[list_index]); + ret = g_strdup(list[list_index]); list_index++; } @@ -3055,7 +3055,7 @@ vshReadline(vshControl *ctl, const char *prompt) if (len > 0 && r[len-1] == '\n') r[len-1] = '\0'; - return vshStrdup(ctl, r); + return g_strdup(r); } #endif /* !WITH_READLINE */ @@ -3095,7 +3095,7 @@ vshInitDebug(vshControl *ctl) /* log file not set from cmdline */ debugEnv = getenv(env); if (debugEnv && *debugEnv) { - ctl->logfile = vshStrdup(ctl, debugEnv); + ctl->logfile = g_strdup(debugEnv); vshOpenLogFile(ctl); } VIR_FREE(env); @@ -3345,7 +3345,7 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) } str = virBufferContentAndReset(&xmlbuf); } else { - str = vshStrdup(ctl, arg); + str = g_strdup(arg); } if (shell) -- 2.21.0

Now that we use g_strdup everywhere, delete vshStrdup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/vsh.c | 12 ------------ tools/vsh.h | 4 ---- 2 files changed, 16 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index baba5ec314..a10a9625e4 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -142,18 +142,6 @@ _vshCalloc(vshControl *ctl, size_t nmemb, size_t size, const char *filename, exit(EXIT_FAILURE); } -char * -_vshStrdup(vshControl *ctl, const char *s, const char *filename, int line) -{ - char *x; - - if (VIR_STRDUP(x, s) >= 0) - return x; - vshError(ctl, _("%s: %d: failed to allocate %lu bytes"), - filename, line, (unsigned long)strlen(s)); - exit(EXIT_FAILURE); -} - int vshNameSorter(const void *a, const void *b) { diff --git a/tools/vsh.h b/tools/vsh.h index 99977af7e3..ad783e24b7 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -473,10 +473,6 @@ void *_vshCalloc(vshControl *ctl, size_t nmemb, size_t sz, #define vshCalloc(_ctl, _nmemb, _sz) \ _vshCalloc(_ctl, _nmemb, _sz, __FILE__, __LINE__) -char *_vshStrdup(vshControl *ctl, const char *s, const char *filename, - int line); -#define vshStrdup(_ctl, _s) _vshStrdup(_ctl, _s, __FILE__, __LINE__) - /* Macros to help dealing with mutually exclusive options. */ /* VSH_EXCLUSIVE_OPTIONS_EXPR: -- 2.21.0

On 10/18/19 7:37 PM, Ján Tomko wrote:
Ján Tomko (10): tools: cmdDomblkinfoGet: reindent parameters virsh: use g_strdup in cmdDomblkinfoGet virsh: use g_strdup in virshDomainGetEditMetadata virsh: getSignalNumber: rename variables virsh: getSignalNumber: use g_autofree virsh: getSignalNumber: use g_strdup tools: vshCommandArgvGetArg: one parameter per line tools: vshCommandArgvGetArg: prefer g_strdup tools: prefer g_strdup to vshStrdup tools: delete vshStrdup
All patches: Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Question: I see that you're pushing the glib chronicles recently. Do you have plans to do the virAsprintf -> g_strdup_printf conversion? I am asking because I started this conversion yesterday, and the amount of calls to be converted is so massive that it's kind of funny. And it's not a work that I'll be able to full commit during the workday. So, if this conversion is under your radar and you want to get it done ASAP (meaning that you'll get it done faster than I'm capable of), let me know and I'll stand down from pursuing that. Thanks, DHB
tools/virsh-checkpoint.c | 4 +-- tools/virsh-domain-monitor.c | 27 ++++++++++---------- tools/virsh-domain.c | 38 ++++++++++++---------------- tools/virsh-nodedev.c | 5 ++-- tools/virsh-pool.c | 30 +++++++++++----------- tools/virsh-snapshot.c | 6 ++--- tools/virsh-volume.c | 11 ++++----- tools/virsh.c | 9 +++---- tools/virt-admin.c | 6 ++--- tools/vsh.c | 48 ++++++++++++++---------------------- tools/vsh.h | 4 --- 11 files changed, 81 insertions(+), 107 deletions(-)

On Sat, Oct 19, 2019 at 07:21:15AM -0300, Daniel Henrique Barboza wrote:
On 10/18/19 7:37 PM, Ján Tomko wrote:
Ján Tomko (10): tools: cmdDomblkinfoGet: reindent parameters virsh: use g_strdup in cmdDomblkinfoGet virsh: use g_strdup in virshDomainGetEditMetadata virsh: getSignalNumber: rename variables virsh: getSignalNumber: use g_autofree virsh: getSignalNumber: use g_strdup tools: vshCommandArgvGetArg: one parameter per line tools: vshCommandArgvGetArg: prefer g_strdup tools: prefer g_strdup to vshStrdup tools: delete vshStrdup
All patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Question: I see that you're pushing the glib chronicles recently. Do you have plans to do the virAsprintf -> g_strdup_printf conversion?
I am asking because I started this conversion yesterday, and the amount of calls to be converted is so massive that it's kind of funny. And it's not a work that I'll be able to full commit during the workday
mprivozn started working on that on Friday using Coccinelle, you need to ask him. Chances are you already converted some usage that was not simply caught by his spatch. Personally I'm working on a branch that does VIR_STRDUP -> g_strdup.
So, if this conversion is under your radar and you want to
https://en.wiktionary.org/wiki/under_the_radar Jano
get it done ASAP (meaning that you'll get it done faster than I'm capable of), let me know and I'll stand down from pursuing that.

On 10/19/19 12:55 PM, Ján Tomko wrote:
On Sat, Oct 19, 2019 at 07:21:15AM -0300, Daniel Henrique Barboza wrote:
On 10/18/19 7:37 PM, Ján Tomko wrote:
Ján Tomko (10): tools: cmdDomblkinfoGet: reindent parameters virsh: use g_strdup in cmdDomblkinfoGet virsh: use g_strdup in virshDomainGetEditMetadata virsh: getSignalNumber: rename variables virsh: getSignalNumber: use g_autofree virsh: getSignalNumber: use g_strdup tools: vshCommandArgvGetArg: one parameter per line tools: vshCommandArgvGetArg: prefer g_strdup tools: prefer g_strdup to vshStrdup tools: delete vshStrdup
All patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Question: I see that you're pushing the glib chronicles recently. Do you have plans to do the virAsprintf -> g_strdup_printf conversion?
I am asking because I started this conversion yesterday, and the amount of calls to be converted is so massive that it's kind of funny. And it's not a work that I'll be able to full commit during the workday
mprivozn started working on that on Friday using Coccinelle, you need to ask him. Chances are you already converted some usage that was not simply caught by his spatch.
Nevermind then. I'll let Michal do that, then he can do it all at once and we'll not duplicate efforts. Thanks for letting me know.
Personally I'm working on a branch that does VIR_STRDUP -> g_strdup.
So, if this conversion is under your radar and you want to
Owned. I meant 'on your radar' and wrote the opposite :P DHB
Jano
get it done ASAP (meaning that you'll get it done faster than I'm capable of), let me know and I'll stand down from pursuing that.

On 10/19/19 5:55 PM, Ján Tomko wrote:
On Sat, Oct 19, 2019 at 07:21:15AM -0300, Daniel Henrique Barboza wrote:
On 10/18/19 7:37 PM, Ján Tomko wrote:
Ján Tomko (10): tools: cmdDomblkinfoGet: reindent parameters virsh: use g_strdup in cmdDomblkinfoGet virsh: use g_strdup in virshDomainGetEditMetadata virsh: getSignalNumber: rename variables virsh: getSignalNumber: use g_autofree virsh: getSignalNumber: use g_strdup tools: vshCommandArgvGetArg: one parameter per line tools: vshCommandArgvGetArg: prefer g_strdup tools: prefer g_strdup to vshStrdup tools: delete vshStrdup
All patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Question: I see that you're pushing the glib chronicles recently. Do you have plans to do the virAsprintf -> g_strdup_printf conversion?
I am asking because I started this conversion yesterday, and the amount of calls to be converted is so massive that it's kind of funny. And it's not a work that I'll be able to full commit during the workday
mprivozn started working on that on Friday using Coccinelle, you need to ask him. Chances are you already converted some usage that was not simply caught by his spatch.
Indeed, I just can't decide whether to drop all checks related to virAsprintf() first and only after that do transition to g_strdup_printf() or do the transition first and then drop all checks related (since the glib function can't fail). https://travis-ci.org/zippy2/libvirt/builds/600171599 https://travis-ci.org/zippy2/libvirt/builds/600597977 I will need to rebase both branches when Jano pushes his VIR_STRDUP branch though. Michal
participants (3)
-
Daniel Henrique Barboza
-
Ján Tomko
-
Michal Privoznik