On Fri, Sep 25, 2020 at 11:15:28AM -0400, Eduardo Habkost wrote:
> On Fri, Sep 25, 2020 at 03:00:51PM +0200, Markus Armbruster wrote:
> > Eduardo Habkost <ehabk...@redhat.com> writes:
> >
> > > On Tue, Sep 22, 2020 at 05:00:47PM -0400, John Snow wrote:
> > >> The edge case is that if the name is '', this expression returns a
> > >> string instead of a bool, which violates our declared type.
> > >>
> > >> Signed-off-by: John Snow <js...@redhat.com>
> > >> ---
> > >> scripts/qapi/gen.py | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> > >> index 9898d513ae..cb2b2655c3 100644
> > >> --- a/scripts/qapi/gen.py
> > >> +++ b/scripts/qapi/gen.py
> > >> @@ -251,7 +251,7 @@ def __init__(self, prefix, what, user_blurb,
> > >> builtin_blurb, pydoc):
> > >>
> > >> @staticmethod
> > >> def _is_user_module(name):
> > >> - return name and not name.startswith('./')
> > >> + return name is not None and not name.startswith('./')
> > >
> > > This changes behavior if name=='', and I guess this is OK, but
> > > I'm not sure.
> >
> > @name is either
> >
> > (1) A module pathname relative to the main module
> >
> > This is a module defined by the user.
> >
> > (2) system module name, starting with './'
> >
> > This is a named system module. We currently have two: './init' in
> > commands.py, and and './emit' in events.py.
> >
> > (3) None
> >
> > This is the (nameless) system module for built-in stuff. It
> > predates (2). Using './builtin' would probably be better now.
> >
> > Note that (1) and (2) are disjoint: relative pathnames do not begin with
> > './'.
> >
> > name='' is not possible, because '' is not a valid pathname.
>
> Thanks! So, the './' prefix is just internal state and never
> visible to the outside, correct? I would use a separate bool
> instead of trying to encode additional state inside the string.
I've found only one place where the './' prefix might be leaking,
and I don't know if it's intentional or not:
Is the name argument to visit_include() supposed to be always
(1), or are './' pathnames allowed too?
--
Eduardo
