[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent.
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent. |
Date: |
Fri, 9 May 2014 16:55:31 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
The Friday 09 May 2014 à 07:36:31 (+0200), Markus Armbruster wrote :
> 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.
>
> [...]
>
Ok. will redo it.