Both function description and function itself mention check for
OOM which can't happen really. There was a bug in glib where
g_strdup_*() might have not aborted on OOM, but we have our own
implementation when dealing with broken glib (see
vir_g_strdup_printf()). Therefore, checking for OOM is redundant
and can never be true.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Let's check whether a boolean --option doesn't have completer or
completer_flags set. These options are just flags and don't
accept any value, thus they can't have any completer.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If a command is an alias, then it can only have .name, .flags and
.alias set and .flags should contain just VSH_CMD_FLAG_ALIAS.
Check if that's the case in self-test.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's now also used in vshCompleteHelpCommand which is outside of the
conditionally compiled code.
Fixes: 80f70c74a7
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Wrap 'vshReadlineCommandGenerator' into a function with proper prototype
to provide a completer for the help command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Make it simple to spot which options of which commands are missing
autocompletion functions by introducing this hidden option.
In the future when we'll have completers for everything this can be also
used as a hard fail so that completers are always added.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't need to validate the real command twice, but it's better to
check that the real command name exists and it's not an alias to prevent
loops.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce a proper flag 'VSH_CMD_FLAG_HIDDEN' for hiding commands from
output so that we can validate that there aren't any loops or
misconfigured commands.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'g_strsplit' to split the strings and then concatenate back when the
escape sequence (',,') is used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a '--split' switch for the 'virsh echo' command and add few test
cases to the virshtest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the need for temporary strings by filling the output buffer
directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Initialize the flags earlier and use VSH_EXCLUSIVE_OPTIONS_VAR to
declare the conflicting options as exclusive.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Note that it's for internal testing use and remove the manpage entry.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove cleanup sections that are no longer needed, as well
as unnecessary 'ret' variables.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Do not use 'arg' which is later used for an allocated string.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
None of them are currently needed to pass our upstream CI, most were
either for ancient clang versions or coverity for silencing false
positives.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
One of the error branches used a plain free where vshCommandFree
was required.
https://bugzilla.redhat.com/show_bug.cgi?id=1943415
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Generated by the following spatch:
@@
expression a, b;
@@
+ b = g_steal_pointer(&a);
- b = a;
... when != a
- a = NULL;
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
These functions are identical. Made using this spatch:
@@
expression path, mode;
@@
- virFileMakePathWithMode(path, mode)
+ g_mkdir_with_parents(path, mode)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The glib implementation doesn't tolerate NULL but in most cases we check
before anyways. The rest of the callers adds a NULL check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
After previous patches neither vshReadlineCommandGenerator() nor
vshReadlineOptionsGenerator() use prefix that user wants to
complete. The argument is marked as unused in both functions.
Drop it then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Firstly, move variable declarations into the inner most block
they are used. Secondly, use for() loop instead of while so that
we don't have to advance loop counter explicitly on 'continue'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The way we currently call completer callbacks is that if we've
found --option that user wants to complete value for and it has
callback set then the callback is called.
And just before that, if no --option to have the value completed
is found or is found and is of boolean type then a list of
--option is generated (for given command).
But these two conditions can never be true at the same time
because boolean type of --options do not accept values. Therefore
the calling of completer callback can be promoted onto the same
level as the --option list generation.
This means that merging of two lists can be dropped to and
completer callback can store its retval directly into @list (but
as shown earlier one of the string lists to merge is always
empty).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Completer callbacks generate all possible outputs ignoring any partial
input (e.g. prefix of a domain name) and then use vshCompleterFilter() to
filter out those strings which don't fit the partial input (prefix).
In contrast, vshReadlineCommandGenerator() does some internal filtering and
only generates completions that match a given prefix. Rather than treating
these scenarios differently, simply generate all possible options and
filter them all at the end.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Completer callbacks generate all possible outputs ignoring any partial
input (e.g. prefix of a domain name) and then use vshCompleterFilter() to
filter out those strings which don't fit the partial input (prefix).
In contrast, vshReadlineOptionsGenerator() does some internal filtering and
only generates completions that match a given prefix. Rather than treating
these scenarios differently, simply generate all possible options and
filter them all at the end.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The vshReadlineParse() function is called whenever user hits
<TAB><TAB>. If there is no command (or a partially written one),
then a list of possible commands is printed to the user. But, if
there is a command then its --options are generated. But
obviously, we can not generate --options if there already is an
--option that's expecting a value. For instance, consider:
virsh # start --domain <TAB><TAB>
In this case we want to call completer for --domain option, but
that's a different story.
Anyway, the way that we currently check whether --options list
should be generated is checking the type of the last --option. If
it isn't DATA, STRING, INT, or ARGV (all these expect a value),
then we can generate --option list. Well, writing the condition
this way is needlessly verbose and also prone to errors (see
d9a320bf97 for example).
We know that boolean type does not require a value. This leaves
us with the only type that was not mentioned yet - VSH_OT_ALIAS.
This is a special type for backwards compatibility and it refers
to another --option which can be just any type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
There are two functions that are used to generate completion
lists: vshReadlineCommandGenerator() for command names and
vshReadlineOptionsGenerator() for --options for given command.
Both return a string list, but may also fail while constructing
it. For that case, they call g_strfreev() explicitly, which is
needless since we have g_auto(GStrv).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The vshReadlineOptionsGenerator() function returns a string list
of all --options for given command. But the way that individual
items on the list are allocated can be written better.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
The aim of vshCompleterFilter() is to take a string list and a
prefix and remove all strings from the list that don't have the
desired prefix. The function is used to filter out those strings
returned by a completer callback that don't correspond with
user's (partial) input. For instance, domain name completer
virshDomainNameCompleter() returns all domain names and then
vshCompleterFilter() refines the list so that only domains with
correct prefix of their name are offered to user. This was a
design choice - it allows us to have shorter completers as they
do not have to copy the list filtering over and over.
Having said all of that, it may happen that a completer does not
return anything (e.g. there is no domain in requested state,
virsh is not connected and thus completer exited early, etc.). In
that case, the string list is NULL and vshCompleterFilter() can
simply return early.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
This saves us explicit call of g_strfreev() in error path.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
We've invented VSH_OT_ALIAS type for --option so that we can
rewrite some --options (e.g. fix spelling). For instance
blkdeviotune command uses this feature heavily:
--options-with-dash are preferred over old
--options_with_underscore. Both versions are supported but only
the new ones (not aliased) are documented and reported in --help.
Except for options completer, which happily put also aliased
versions in front of user's eyes.
Note, there is a second (gross) way we use aliases: to rewrite
options from --oldoption to --newoption=value (for instance
--shareable option of attach-disk is an alias of
--mode=shareable). And just like with the previous group - don't
generate them into the list of possible options.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
There are few cases where STREQLEN() is called like this:
STREQLEN(var, string, strlen(string))
which is the same as STRPREFIX(var, string). Use STRPREFIX()
because it is more obvious what the check is doing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
If user is trying to auto complete a value that contains a space,
they have two options: use backslash to escape space or use
quotes, like this:
virsh # start --domain "domain with space<TAB>
However, in this case our tokenizer sees imbalance in (double)
quotes: there is a starting one that's missing its companion.
Well, that's obvious - user is still in process of writing the
command. What we need to do in this case is to ignore the
imbalance and return success (from the tokenizer) - readline will
handle closing the quote properly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The way that auto completion works currently is that user's input
is parsed, and then we try to find the first --option (in the
parsed structure) that has the same value as user's input around
where <TAB> was pressed. For instance, for the following input:
virsh # command --arg1 hello --arg2 world<TAB>
we will see "world" as text that user is trying to autocomplete
(this is affected by rl_basic_word_break_characters which
readline uses internally to break user's input into individual
words) and find that it is --arg2 that user is trying to
autocomplete. So far so good, for this naive approach. But
consider the following example:
virsh # command --arg1 world --arg2 world<TAB>
Here, both arguments have the same value and because we see
"world" as text that user is trying to autocomplete we would
think that it is --arg1 that user wants to autocomplete. This is
obviously wrong.
Fortunately, readline stores the current position of cursor (into
rl_point) and we can use that when parsing user's input: whenever
we reach a position that matches the cursor then we know that
that is the place where <TAB> was pressed and hence that is the
--option that user wants to autocomplete. Readline stores the
cursor position as offset (numbered from 1) from the beginning of
user's input. We store this input into @parser->pos initially,
but then advance it as we tokenize it. Therefore, what we need is
to store the original position too.
Thanks to Martin who helped me with this.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The way our completer callbacks work is that they return all
possible candidates and then vshCompleterFilter() is called to
prune the list of all candidates removing those which don't match
user's input. This allows us to have simpler completer callbacks
as their only job is to fetch all possible candidates.
Anyway, if the completion candidate we're returning contains a
space, it has to be escaped (shell like escaping), unless there
is already a quote character (single quote or double quote).
But ordering is critical. Completer callback returns string
without any escaping, but the filter function sees the user input
escaped. For instance, if user's input is "domain with
space<TAB>" then the filtering function gets "domain\ with\
space" as user's input but completer returns "domain with space".
Since these two strings don't match the filtering function
removes this candidate from the list. What we need to do is to
escape strings before calling the filtering function. This way,
the filtering function will see two same strings.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In next commit the block that does escaping of returned string
will be brought into this block. But both contain variable @buf
and use it in different contexts. Rename @buf from @state == 0
block to @line which reflects its purpose better.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Instead of freeing @partial and @buf explicitly, we can use
g_auto*() to do that automatically.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
On readline completion vshReadlineCompletion() is called which
does nothing more than calling rl_completion_matches() with
vshReadlineParse() as a callback. This means, that
vshReadlineParse() is called repeatedly, each time returning next
completion candidate, until it returns NULL which is interpreted
as the end of the list of candidates.
The function takes two parameters: @text which is a portion of
input line around cursor when TAB was pressed, and @state. The
@state is an integer that is zero on the very first call and
non-zero on each subsequent call (in fact, readline does @state++
on each call).
Anyway, the idea is that the callback gets the whole list of
candidates on @state == 0 and returns one candidate at each call.
And this is what vshReadlineParse() is doing but some variables
(@partial, @cmd and @opt) are really used only in the @state == 0
case but declared for whole function. We can limit their scope by
declaring them inside the @state == 0 body which also means that
they don't have to be static anymore.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
A backslash is the way we escape characters in virsh. For
instance:
virsh # start domain\ with\ long\ name
For readline completion, we do not want to get four separate
words ("domain", "with", "long", "name"). This means, that we
can't use virBufferEscapeShell() because it doesn't escape spaces
the way we want.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This variable is unused since introduction of the function in
v0.8.5~150.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function can't fail so there's no need to return a value or check it
in the callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tim Wiederhake