Re: [PATCH] qom: Check for wellformed id in user_creatable_add_type()

[Paolo Bonzini]

On 02/03/21 18:16, Kevin Wolf wrote:
> Most code paths for creating a user creatable object go through
> QemuOpts, which ensures that the provided 'id' option is actually a
> valid identifier.
>
> However, there are some code paths that don't go through QemuOpts:
> qemu-storage-daemon --object (since commit 8db1efd3) and QMP object-add
> (since it was first introduced in commit cff8b2c6). We need to have the
> same validity check for those, too.
>
> This adds the check and makes it print the same error message as
> QemuOpts on failure.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> This makes sense even without the -object QAPIfication, so no reason to
> wait for v3 of that series to get this fixed.

It needs a check for id != NULL, but no big deal so I added it.

Thanks,

Paolo

>   qom/object_interfaces.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 1e9ad6f08a..515ca4557e 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -8,6 +8,7 @@
>   #include "qapi/qobject-input-visitor.h"
>   #include "qom/object_interfaces.h"
>   #include "qemu/help_option.h"
> +#include "qemu/id.h"
>   #include "qemu/module.h"
>   #include "qemu/option.h"
>   #include "qapi/opts-visitor.h"
> @@ -41,11 +42,19 @@ Object *user_creatable_add_type(const char *type, const 
> char *id,
>                                   const QDict *qdict,
>                                   Visitor *v, Error **errp)
>   {
> +    ERRP_GUARD();
>       Object *obj;
>       ObjectClass *klass;
>       const QDictEntry *e;
>       Error *local_err = NULL;
>   
> +    if (!id_wellformed(id)) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
> +        error_append_hint(errp, "Identifiers consist of letters, digits, "
> +                          "'-', '.', '_', starting with a letter.\n");
> +        return NULL;
> +    }
> +
>       klass = object_class_by_name(type);
>       if (!klass) {
>           error_setg(errp, "invalid object type: %s", type);