Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent.

[Markus Armbruster]

Benoît Canet <address@hidden> writes:

> The Thursday 08 May 2014 à 20:30:33 (+0200), Markus Armbruster wrote :
[...]
>> There are two reasons to detect cycles.  The technical one is preventing
>> infinite expansion.  No longer applies with idempotent include.  The
>> other, more abstract one is rejecting nonsensical use of the include
>> directive.  I think that one still applies.
[...]
>> > @@ -102,17 +102,16 @@ class QAPISchema:
>> >                                          'Expected a file name (string), 
>> > got: %s'
>> >                                          % include)
>> >                  include_path = os.path.join(self.input_dir, include)
>> > -                if any(include_path == elem[1]
>> > -                       for elem in self.include_hist):
>> > -                    raise QAPIExprError(expr_info, "Inclusion loop for %s"
>> > -                                        % include)
>> > +                # make include idempotent by skipping further includes
>> > +                if include_path in [x[1] for x in  include_hist]:
>> > +                    continue
>> 
>> Loses cycle detection.
>
> It simply also skip cycle includes. If cycle are skipped then cannot do no
> harm.

Your argument is based exclusively on the technical reason to detect
cycles: cycles need to be caught because they cause infinite recursion.
Since there is no infinite recursion with idempotent include, cycles are
just fine.

I'm arguing from a more abstract point of view: cycles should be
rejected because they're nonsensical.  The fact that they can cause
infinite recursion is an implementation detail.  Even without infinite
recursion, they're just as nonsensical as ever.

[...]