Eric Blake <ebl...@redhat.com> writes: > On 09/29/2014 02:38 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> Now that we have a way to validate every type, we can also be >>> stricter about enforcing that callers that want to bypass >>> type safety in generated code. Prior to this patch, it didn't >>> matter what value was associated with the key 'gen', but it >>> looked odd that 'gen':'yes' could result in bypassing the >>> generated code. These changes also enforce the changes made >>> earlier in the series for documentation and consolidation of >>> using '**' as the wildcard type. >> >> Perhaps it's worth mentioning that the schema has always used 'gen': >> 'no'. >> >> That said, 'no' is silly. The natural JSON for a flag is false / true! >> Since you're touching it anyway, consider making it an optional boolean >> defaulting to false. Same for the other silly 'no': success-response. > > I'd love to, except that the .json parser does not yet allow literal > true/false JSON keywords. Fam had a patch back in May that would fix > that; maybe I'll fold in his patch to my v5 as a prereq patch: > > https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03916.html
Your choice. I certainly don't want the silly 'no' block your series. >>> @@ -256,13 +256,13 @@ def discriminator_find_enum_define(expr): >>> return find_enum(discriminator_type) >>> >>> def check_type(expr_info, source, data, allow_array = False, >>> - allowed_names = [], allow_dict = True): >>> + allowed_names = [], allow_dict = True, allow_star = False): >>> global all_types >>> >>> if data is None: >>> return >>> >>> - if data == '**': >>> + if allow_star and data == '**': >>> return > > [1] > >>> >>> # Check if array type for data is okay >>> @@ -278,6 +278,10 @@ def check_type(expr_info, source, data, allow_array = >>> False, >>> >>> # Check if type name for data is okay >>> if isinstance(data, basestring): >>> + if data == '**': >>> + raise QAPIExprError(expr_info, >>> + "%s uses '**' but did not request >>> 'gen':'no'" >>> + % source) >>> if not data in all_types: >>> raise QAPIExprError(expr_info, >>> "%s references unknown type '%s'" >> >> I'm blind: I can't see where this error gets suppressed when we have >> 'gen': 'no'. > > 'gen':'no' triggers the caller to pass allow_star=True to check_type > [2]; then at point [1] we short-circuit and exit if '**' was passed. > Therefore, if we get here and still have '**', then allow_star is still > false, which means 'gen':'no' was not passed and it is user error. Got it, thanks! [...]
