On 03/03/2017 09:32 AM, Markus Armbruster wrote:
> The string input visitor tries to cope with null input. Null input
> isn't used anywhere, and isn't covered by tests. Unsurprisingly, it
> doesn't fully work: start_list() crashes because it passes the input
> via parse_str() to strtoll() unchecked.
>
> Make string_input_visitor_new() assert its argument isn't null, and
> drop the code trying to deal with null input.
>
> The opts visitor crashes when you try to actually visit something with
> null input. Make opts_visitor_new() assert its argument isn't null,
> mostly for clarity.
>
> qobject_input_visitor_new() already asserts its argument isn't null.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> qapi/opts-visitor.c | 1 +
> qapi/string-input-visitor.c | 54 ++++++++++++++-------------------------------
> 2 files changed, 18 insertions(+), 37 deletions(-)
>
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index a0a7c0e..c50dc4b 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -528,6 +528,7 @@ opts_visitor_new(const QemuOpts *opts)
> {
> OptsVisitor *ov;
>
> + assert(opts);
> ov = g_malloc0(sizeof *ov);
>
> ov->visitor.type = VISITOR_INPUT;
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 1a855c5..f126cd9 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -182,12 +182,6 @@ static void parse_type_int64(Visitor *v, const char *name, int64_t *obj,
> {
> StringInputVisitor *siv = to_siv(v);
>
> - if (!siv->string) {
> - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> - "integer");
> - return;
> - }
> -
> if (parse_str(siv, name, errp) < 0) {
> return;
> }
> @@ -242,13 +236,7 @@ static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,
> Error *err = NULL;
> uint64_t val;
>
> - if (siv->string) {
> - parse_option_size(name, siv->string, &val, &err);
> - } else {
> - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> - "size");
> - return;
> - }
> + parse_option_size(name, siv->string, &val, &err);
> if (err) {
> error_propagate(errp, err);
> return;
> @@ -262,19 +250,17 @@ static void parse_type_bool(Visitor *v, const char *name, bool *obj,
> {
> StringInputVisitor *siv = to_siv(v);
>
> - if (siv->string) {
> - if (!strcasecmp(siv->string, "on") ||
> - !strcasecmp(siv->string, "yes") ||
> - !strcasecmp(siv->string, "true")) {
> - *obj = true;
> - return;
> - }
> - if (!strcasecmp(siv->string, "off") ||
> - !strcasecmp(siv->string, "no") ||
> - !strcasecmp(siv->string, "false")) {
> - *obj = false;
> - return;
> - }
> + if (!strcasecmp(siv->string, "on") ||
> + !strcasecmp(siv->string, "yes") ||
> + !strcasecmp(siv->string, "true")) {
> + *obj = true;
> + return;
> + }
> + if (!strcasecmp(siv->string, "off") ||
> + !strcasecmp(siv->string, "no") ||
> + !strcasecmp(siv->string, "false")) {
> + *obj = false;
> + return;
> }
>
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> @@ -285,13 +271,8 @@ static void parse_type_str(Visitor *v, const char *name, char **obj,
> Error **errp)
> {
> StringInputVisitor *siv = to_siv(v);
> - if (siv->string) {
> - *obj = g_strdup(siv->string);
> - } else {
> - *obj = NULL;
> - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> - "string");
> - }
> +
> + *obj = g_strdup(siv->string);
> }
>
> static void parse_type_number(Visitor *v, const char *name, double *obj,
> @@ -302,10 +283,8 @@ static void parse_type_number(Visitor *v, const char *name, double *obj,
> double val;
>
> errno = 0;
> - if (siv->string) {
> - val = strtod(siv->string, &endp);
> - }
> - if (!siv->string || errno || endp == siv->string || *endp) {
> + val = strtod(siv->string, &endp);
> + if (errno || endp == siv->string || *endp) {
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> "number");
> return;
> @@ -327,6 +306,7 @@ Visitor *string_input_visitor_new(const char *str)
> {
> StringInputVisitor *v;
>
> + assert(str);
> v = g_malloc0(sizeof(*v));
>
> v->visitor.type = VISITOR_INPUT;
>