Re: [Qemu-devel] [PATCH v5 21/28] qapi: Require valid names

[Markus Armbruster]

Eric Blake <address@hidden> writes:

[...]
> +valid_characters = set(string.ascii_letters + string.digits + '.' + '-' + 
> '_')
> +def check_name(expr_info, source, name, allow_optional = False):
> +    membername = name
> +
> +    if not isinstance(name, str):
> +        raise QAPIExprError(expr_info,
> +                            "%s requires a string name" % source)
> +    if name == '**':
> +        return
> +    if name.startswith('*'):
> +        membername = name[1:]
> +        if not allow_optional:
> +            raise QAPIExprError(expr_info,
> +                                "%s does not allow optional name '%s'"
> +                                % (source, name))
> +    if not set(membername) <= valid_characters:
> +        raise QAPIExprError(expr_info,
> +                            "%s uses invalid name '%s'" % (source, name))
> +

This accepts names starting with a digit.  Sure we generate valid C
identifiers for such beauties?

qapi-code-gen.txt:

    Downstream vendors may add extensions; such extensions should begin
    with a prefix matching "__RFQDN_" (for the reverse-fully-qualified-
    domain-name of the vendor), even if the rest of the command or field
    name uses dash (example: __com.redhat_drive-mirror).  Other than the
    dots used in RFQDN of a downstream extension, all command, type, and
    field names should begin with a letter, and contain only ASCII
    letters, numbers, dash, and underscore.

One, I think "all command, type, and field names" is too narrow, what
about event names, or enumeration value names?  Suggest say just "all
names".

Two, "letters, digits, dash, and underscore", please.

Three, I think check_name() should enforce "starts with letter or '_'".

[...]