Re: [PATCH] qom/object: Limit type names to alphanumerical and some few special characters

[Thomas Huth <thuth@redhat.com>]

On 14/11/2023 14.21, Peter Maydell wrote:
> On Tue, 14 Nov 2023 at 13:05, Thomas Huth <thuth@redhat.com> wrote:
>>
>> QOM names currently don't have any enforced naming rules. This
>> can be problematic, e.g. when they are used on the command line
>> for the "-device" option (where the comma is used to separate
>> properties). To avoid that such problematic type names come in
>> again, let's restrict the set of acceptable characters during the
>> type registration.
>>
>> Ideally, we'd apply here the same rules as for QAPI, i.e. all type
>> names should begin with a letter, and contain only ASCII letters,
>> digits, hyphen, and underscore. However, we already have so many
>> pre-existing types like:
>>
>>      486-x86_64-cpu
>>      cfi.pflash01
>>      power5+_v2.1-spapr-cpu-core
>>      virt-2.6-machine::hotplug-handler
>>      aspeed.i2c.slave::vmstate-if
>>      pc-i440fx-3.0-machine::nmi
> 
> I think all these '::' are specifically interface types --
> see type_initialize_interface(), which constructs the
> interface type name by gluing together the class name and
> the interface name with a '::'. The rule we ought to be
> requiring for ':' I think is "no : in the type name, unless
> it is the one generated by type_initialize_interface()".
> 
> I think we could do that by having the type_name_is_valid()
> checks done in:
>   * type_initialize_interface(), on ti->name and interface_type->name
>   * type_register_internal(), on info->name
> 
> If we do that, can we take ':' out of the list of characters
> we permit in type_name_is_valid() ?

Thanks, that's a very good idea! Actually, after looking at the code for a 
while, I think it should even be enough to add a check to 
type_register_internal(), since type_initialize_interface() is rather used 
with base types and interface types that should have been registered already 
via type_register...() before.

There just seem to be two stragglers left:

1) #define TYPE_DUMMY "qemu:dummy" in tests/unit/test-io-task.c ...
    easy to fix, it's just a unit test anyway

2) #define TYPE_RAM_DISCARD_MANAGER "qemu:ram-discard-manager"
    in include/exec/memory.h ... I believe it should be OK to
    simply rename it, since it's about an interface type...
    Or do we use these interface names in migration streams, too?

I'll try to come up with some patches to rename them ... let's see how it 
goes...

  Thomas