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

["Benoît Canet" <benoit.canet@irqsave.net>]

The Friday 09 May 2014 à 07:36:31 (+0200), Markus Armbruster wrote :
> Benoît Canet <benoit.canet@irqsave.net> 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.
> 
> [...]
> 

Ok. will redo it.