12:14 p.m.
Except for vshCommandParse.
-EPARSE
Ján Tomko (5):
vsh: do not cast away const
vsh: cmdEcho: use separate variable for argument
vsh: use g_auto where possible
vsh: remove pointless cleanup labels
vsh: use g_clear_pointer
tools/vsh.c | 91 ++++++++++++++++++-----------------------------------
1 file changed, 31 insertions(+), 60 deletions(-)
--
2.31.1
12:14 p.m.
New subject: [libvirt PATCH 1/5] vsh: do not cast away const
Instead of using the same variable to store either a const pointer
or an allocated string, always make a copy.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 123284c636..3bbaecd2ea 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -318,7 +318,7 @@ vshCmddefCheckInternals(vshControl *ctl,
case VSH_OT_ALIAS: {
size_t j;
- char *name = (char *)opt->help; /* cast away const */
+ g_autofree char *name = NULL;
char *p;
if (opt->flags || !opt->help) {
@@ -326,15 +326,16 @@ vshCmddefCheckInternals(vshControl *ctl,
opt->name, cmd->name);
return -1; /* alias options are tracked by the original name */
}
- if ((p = strchr(name, '=')))
- name = g_strndup(name, p - name);
+ if ((p = strchr(opt->help, '=')))
+ name = g_strndup(opt->help, p - opt->help);
+ else
+ name = g_strdup(opt->help);
for (j = i + 1; cmd->opts[j].name; j++) {
if (STREQ(name, cmd->opts[j].name) &&
cmd->opts[j].type != VSH_OT_ALIAS)
break;
}
- if (name != opt->help) {
- VIR_FREE(name);
+ if (p) {
/* If alias comes with value, replacement must not be bool */
if (cmd->opts[j].type == VSH_OT_BOOL) {
vshError(ctl, _("alias '%s' of command '%s' has
mismatched alias type"),
--
2.31.1
2:45 a.m.
New subject: [libvirt PATCH 1/5] vsh: do not cast away const
On Tue, Aug 10, 2021 at 19:14:39 +0200, Ján Tomko wrote:
Instead of using the same variable to store either a const pointer
or an allocated string, always make a copy.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
12:14 p.m.
New subject: [libvirt PATCH 2/5] vsh: cmdEcho: use separate variable for argument
Do not use 'arg' which is later used for an allocated string.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 3bbaecd2ea..bf32a8dc22 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3199,17 +3199,16 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
char *str;
g_auto(virBuffer) xmlbuf = VIR_BUFFER_INITIALIZER;
-
- arg = opt->data;
+ const char *curr = opt->data;
if (count)
virBufferAddChar(&buf, ' ');
if (xml) {
- virBufferEscapeString(&xmlbuf, "%s", arg);
+ virBufferEscapeString(&xmlbuf, "%s", curr);
str = virBufferContentAndReset(&xmlbuf);
} else {
- str = g_strdup(arg);
+ str = g_strdup(curr);
}
if (shell)
--
2.31.1
2:50 a.m.
New subject: [libvirt PATCH 2/5] vsh: cmdEcho: use separate variable for argument
On Tue, Aug 10, 2021 at 19:14:40 +0200, Ján Tomko wrote:
Do not use 'arg' which is later used for an allocated
string.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
There's quite some room for rewriting the code to even avoid the use of
temps and string copies though.
12:14 p.m.
New subject: [libvirt PATCH 3/5] vsh: use g_auto where possible
Excluding vshCommandParse.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 46 ++++++++++++++--------------------------------
1 file changed, 14 insertions(+), 32 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index bf32a8dc22..71fbb7401f 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -122,7 +122,7 @@ int
vshStringToArray(const char *str,
char ***array)
{
- char *str_copied = g_strdup(str);
+ g_autofree char *str_copied = g_strdup(str);
char *str_tok = NULL;
char *tmp;
unsigned int nstr_tokens = 0;
@@ -163,7 +163,6 @@ vshStringToArray(const char *str,
arr[nstr_tokens++] = g_strdup(str_tok);
*array = arr;
- VIR_FREE(str_copied);
return nstr_tokens;
}
@@ -434,7 +433,7 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char
*name,
{
size_t i;
const vshCmdOptDef *ret = NULL;
- char *alias = NULL;
+ g_autofree char *alias = NULL;
if (STREQ(name, helpopt.name))
return &helpopt;
@@ -481,7 +480,6 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char
*name,
cmd->name, name);
}
cleanup:
- VIR_FREE(alias);
return ret;
}
@@ -1814,7 +1812,7 @@ void
vshDebug(vshControl *ctl, int level, const char *format, ...)
{
va_list ap;
- char *str;
+ g_autofree char *str = NULL;
/* Aligning log levels to that of libvirt.
* Traces with levels >= user-specified-level
@@ -1831,14 +1829,13 @@ vshDebug(vshControl *ctl, int level, const char *format, ...)
str = g_strdup_vprintf(format, ap);
va_end(ap);
fputs(str, stdout);
- VIR_FREE(str);
}
void
vshPrintExtra(vshControl *ctl, const char *format, ...)
{
va_list ap;
- char *str;
+ g_autofree char *str = NULL;
if (ctl && ctl->quiet)
return;
@@ -1847,7 +1844,6 @@ vshPrintExtra(vshControl *ctl, const char *format, ...)
str = g_strdup_vprintf(format, ap);
va_end(ap);
fputs(str, stdout);
- VIR_FREE(str);
}
@@ -1855,13 +1851,12 @@ void
vshPrint(vshControl *ctl G_GNUC_UNUSED, const char *format, ...)
{
va_list ap;
- char *str;
+ g_autofree char *str = NULL;
va_start(ap, format);
str = g_strdup_vprintf(format, ap);
va_end(ap);
fputs(str, stdout);
- VIR_FREE(str);
}
@@ -1960,7 +1955,7 @@ void
vshError(vshControl *ctl, const char *format, ...)
{
va_list ap;
- char *str;
+ g_autofree char *str = NULL;
if (ctl != NULL) {
va_start(ap, format);
@@ -1980,7 +1975,6 @@ vshError(vshControl *ctl, const char *format, ...)
fprintf(stderr, "%s\n", NULLSTR(str));
fflush(stderr);
- VIR_FREE(str);
}
@@ -2186,7 +2180,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char
*msg_format,
va_list ap)
{
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
- char *str = NULL;
+ g_autofree char *str = NULL;
size_t len;
const char *lvl = "";
g_autoptr(GDateTime) now = g_date_time_new_now_local();
@@ -2237,13 +2231,11 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char
*msg_format,
if (safewrite(ctl->log_fd, str, len) < 0)
goto error;
- VIR_FREE(str);
return;
error:
vshCloseLogFile(ctl);
vshError(ctl, "%s", _("failed to write the log file"));
- VIR_FREE(str);
}
/**
@@ -2359,7 +2351,7 @@ vshAskReedit(vshControl *ctl,
char *
vshEditWriteToTempFile(vshControl *ctl, const char *doc)
{
- char *ret;
+ g_autofree char *ret = NULL;
const char *tmpdir;
int fd;
@@ -2370,7 +2362,6 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc)
if (fd == -1) {
vshError(ctl, _("g_mkstemp_full: failed to create temporary file:
%s"),
g_strerror(errno));
- VIR_FREE(ret);
return NULL;
}
@@ -2379,14 +2370,12 @@ vshEditWriteToTempFile(vshControl *ctl, const char *doc)
ret, g_strerror(errno));
VIR_FORCE_CLOSE(fd);
unlink(ret);
- VIR_FREE(ret);
return NULL;
}
if (VIR_CLOSE(fd) < 0) {
vshError(ctl, _("close: %s: failed to write or close temporary file:
%s"),
ret, g_strerror(errno));
unlink(ret);
- VIR_FREE(ret);
return NULL;
}
@@ -2406,7 +2395,7 @@ int
vshEditFile(vshControl *ctl, const char *filename)
{
const char *editor;
- virCommand *cmd;
+ g_autoptr(virCommand) cmd = NULL;
int ret = -1;
int outfd = STDOUT_FILENO;
int errfd = STDERR_FILENO;
@@ -2450,7 +2439,6 @@ vshEditFile(vshControl *ctl, const char *filename)
ret = 0;
cleanup:
- virCommandFree(cmd);
return ret;
}
@@ -2806,10 +2794,10 @@ vshReadlineCharIsQuoted(char *line, int idx)
static int
vshReadlineInit(vshControl *ctl)
{
- char *userdir = NULL;
+ g_autofree char *userdir = NULL;
int max_history = 500;
int ret = -1;
- char *histsize_env = NULL;
+ g_autofree char *histsize_env = NULL;
const char *histsize_str = NULL;
const char *break_characters = " \t\n`@$><=;|&{(";
const char *quote_characters = "\"'";
@@ -2856,8 +2844,6 @@ vshReadlineInit(vshControl *ctl)
ret = 0;
cleanup:
- VIR_FREE(userdir);
- VIR_FREE(histsize_env);
return ret;
}
@@ -2940,7 +2926,7 @@ static int
vshInitDebug(vshControl *ctl)
{
const char *debugEnv;
- char *env = NULL;
+ g_autofree char *env = NULL;
if (ctl->debug == VSH_DEBUG_DEFAULT) {
env = g_strdup_printf("%s_DEBUG", ctl->env_prefix);
@@ -2957,7 +2943,6 @@ vshInitDebug(vshControl *ctl)
ctl->debug = debug;
}
}
- VIR_FREE(env);
}
if (ctl->logfile == NULL) {
@@ -2969,7 +2954,6 @@ vshInitDebug(vshControl *ctl)
ctl->logfile = g_strdup(debugEnv);
vshOpenLogFile(ctl);
}
- VIR_FREE(env);
}
return 0;
@@ -3186,7 +3170,7 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
bool err = false;
int count = 0;
const vshCmdOpt *opt = NULL;
- char *arg;
+ g_autofree char *arg = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
if (vshCommandOptBool(cmd, "shell"))
@@ -3197,7 +3181,7 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
err = true;
while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
- char *str;
+ g_autofree char *str = NULL;
g_auto(virBuffer) xmlbuf = VIR_BUFFER_INITIALIZER;
const char *curr = opt->data;
@@ -3216,7 +3200,6 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
else
virBufferAdd(&buf, str, -1);
count++;
- VIR_FREE(str);
}
arg = virBufferContentAndReset(&buf);
@@ -3226,7 +3209,6 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd)
else
vshPrint(ctl, "%s", arg);
}
- VIR_FREE(arg);
return true;
}
--
2.31.1
2:40 a.m.
New subject: [libvirt PATCH 3/5++] fixup: vshInitDebug
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 1f384d4ea6..f9600bafba 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2915,10 +2915,9 @@ static int
vshInitDebug(vshControl *ctl)
{
const char *debugEnv;
- g_autofree char *env = NULL;
if (ctl->debug == VSH_DEBUG_DEFAULT) {
- env = g_strdup_printf("%s_DEBUG", ctl->env_prefix);
+ g_autofree char *env = g_strdup_printf("%s_DEBUG",
ctl->env_prefix);
/* log level not set from commandline, check env variable */
debugEnv = getenv(env);
@@ -2935,7 +2934,7 @@ vshInitDebug(vshControl *ctl)
}
if (ctl->logfile == NULL) {
- env = g_strdup_printf("%s_LOG_FILE", ctl->env_prefix);
+ g_autofree char *env = g_strdup_printf("%s_LOG_FILE",
ctl->env_prefix);
/* log file not set from cmdline */
debugEnv = getenv(env);
--
2.31.1
3:01 a.m.
New subject: [libvirt PATCH 3/5++] fixup: vshInitDebug
On Wed, Aug 11, 2021 at 09:40:13 +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
12:14 p.m.
New subject: [libvirt PATCH 4/5] vsh: remove pointless cleanup labels
Remove cleanup sections that are no longer needed, as well
as unnecessary 'ret' variables.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 71fbb7401f..f4b555fb8b 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2396,7 +2396,6 @@ vshEditFile(vshControl *ctl, const char *filename)
{
const char *editor;
g_autoptr(virCommand) cmd = NULL;
- int ret = -1;
int outfd = STDOUT_FILENO;
int errfd = STDERR_FILENO;
@@ -2434,12 +2433,9 @@ vshEditFile(vshControl *ctl, const char *filename)
if (virCommandRunAsync(cmd, NULL) < 0 ||
virCommandWait(cmd, NULL) < 0) {
vshReportError(ctl);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
char *
@@ -2796,7 +2792,6 @@ vshReadlineInit(vshControl *ctl)
{
g_autofree char *userdir = NULL;
int max_history = 500;
- int ret = -1;
g_autofree char *histsize_env = NULL;
const char *histsize_str = NULL;
const char *break_characters = " \t\n`@$><=;|&{(";
@@ -2821,12 +2816,12 @@ vshReadlineInit(vshControl *ctl)
if ((histsize_str = getenv(histsize_env))) {
if (virStrToLong_i(histsize_str, NULL, 10, &max_history) < 0) {
vshError(ctl, _("Bad $%s value."), histsize_env);
- goto cleanup;
+ return -1;
} else if (max_history > HISTSIZE_MAX || max_history < 0) {
vshError(ctl, _("$%s value should be between 0 "
"and %d"),
histsize_env, HISTSIZE_MAX);
- goto cleanup;
+ return -1;
}
}
stifle_history(max_history);
@@ -2841,10 +2836,7 @@ vshReadlineInit(vshControl *ctl)
ctl->historyfile = g_strdup_printf("%s/history", ctl->historydir);
read_history(ctl->historyfile);
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
static void
--
2.31.1
3:02 a.m.
New subject: [libvirt PATCH 4/5] vsh: remove pointless cleanup labels
On Tue, Aug 10, 2021 at 19:14:42 +0200, Ján Tomko wrote:
Remove cleanup sections that are no longer needed, as well
as unnecessary 'ret' variables.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
12:14 p.m.
New subject: [libvirt PATCH 5/5] vsh: use g_clear_pointer
Replace remaining uses of VIR_FREE with g_clear_pointer.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index f4b555fb8b..1f384d4ea6 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2253,10 +2253,7 @@ vshCloseLogFile(vshControl *ctl)
g_strerror(errno));
}
- if (ctl->logfile) {
- VIR_FREE(ctl->logfile);
- ctl->logfile = NULL;
- }
+ g_clear_pointer(&ctl->logfile, g_free);
}
#ifndef WIN32
@@ -2852,8 +2849,8 @@ vshReadlineDeinit(vshControl *ctl)
}
}
- VIR_FREE(ctl->historydir);
- VIR_FREE(ctl->historyfile);
+ g_clear_pointer(&ctl->historydir, g_free);
+ g_clear_pointer(&ctl->historyfile, g_free);
}
char *
--
2.31.1
3:02 a.m.
New subject: [libvirt PATCH 5/5] vsh: use g_clear_pointer
On Tue, Aug 10, 2021 at 19:14:43 +0200, Ján Tomko wrote:
Replace remaining uses of VIR_FREE with g_clear_pointer.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tools/vsh.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
1228
days inactive
1229
days old
11 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa