diff mbox series

[25/25] qapi: Improve source file read error handling

Message ID 20190924132830.15835-26-armbru@redhat.com
State New
Headers show
Series qapi: Pay back some frontend technical debt | expand

Commit Message

Markus Armbruster Sept. 24, 2019, 1:28 p.m. UTC
qap-gen.py crashes when it can't open the main schema file, and when
it can't read from any schema file.  Lazy.

Change QAPISchema.__init__() to take a file name instead of a file
object.  Move the open code from _include() to __init__(), so it's
used for the main schema file, too.

Move the read into the try for good measure, and rephrase the error
message.

Reporting open or read failure for the main schema file needs a
QAPISourceInfo representing "no source".  Make QAPISourceInfo cope
with fname=None.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/common.py                | 46 +++++++++++++++------------
 tests/qapi-schema/include-no-file.err |  2 +-
 2 files changed, 27 insertions(+), 21 deletions(-)

Comments

Eric Blake Sept. 24, 2019, 7:57 p.m. UTC | #1
On 9/24/19 8:28 AM, Markus Armbruster wrote:
> qap-gen.py crashes when it can't open the main schema file, and when

qapi-gen.py

> it can't read from any schema file.  Lazy.
> 
> Change QAPISchema.__init__() to take a file name instead of a file
> object.  Move the open code from _include() to __init__(), so it's
> used for the main schema file, too.
> 
> Move the read into the try for good measure, and rephrase the error
> message.
> 
> Reporting open or read failure for the main schema file needs a
> QAPISourceInfo representing "no source".  Make QAPISourceInfo cope
> with fname=None.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi/common.py                | 46 +++++++++++++++------------
>  tests/qapi-schema/include-no-file.err |  2 +-
>  2 files changed, 27 insertions(+), 21 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster Sept. 24, 2019, 8:59 p.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 9/24/19 8:28 AM, Markus Armbruster wrote:
>> qap-gen.py crashes when it can't open the main schema file, and when
>
> qapi-gen.py

Will fix.

>> it can't read from any schema file.  Lazy.
>> 
>> Change QAPISchema.__init__() to take a file name instead of a file
>> object.  Move the open code from _include() to __init__(), so it's
>> used for the main schema file, too.
>> 
>> Move the read into the try for good measure, and rephrase the error
>> message.
>> 
>> Reporting open or read failure for the main schema file needs a
>> QAPISourceInfo representing "no source".  Make QAPISourceInfo cope
>> with fname=None.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  scripts/qapi/common.py                | 46 +++++++++++++++------------
>>  tests/qapi-schema/include-no-file.err |  2 +-
>>  2 files changed, 27 insertions(+), 21 deletions(-)
>> 
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
diff mbox series

Patch

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c38e7cf27d..0c169a640e 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -53,7 +53,12 @@  class QAPISourceInfo(object):
         return info
 
     def loc(self):
-        return '%s:%d' % (self.fname, self.line)
+        if self.fname is None:
+            return sys.argv[0]
+        ret = self.fname
+        if self.line is not None:
+            ret += ':%d' % self.line
+        return ret
 
     def in_defn(self):
         if self.defn_name:
@@ -382,14 +387,26 @@  class QAPIDoc(object):
 
 class QAPISchemaParser(object):
 
-    def __init__(self, fp, previously_included=[], incl_info=None):
-        self.fname = fp.name
-        previously_included.append(os.path.abspath(fp.name))
-        self.src = fp.read()
+    def __init__(self, fname, previously_included=[], incl_info=None):
+        previously_included.append(os.path.abspath(fname))
+
+        try:
+            if sys.version_info[0] >= 3:
+                fp = open(fname, 'r', encoding='utf-8')
+            else:
+                fp = open(fname, 'r')
+            self.src = fp.read()
+        except IOError as e:
+            raise QAPISemError(incl_info or QAPISourceInfo(None, None, None),
+                               "can't read %s file '%s': %s"
+                               % ("include" if incl_info else "schema",
+                                  fname,
+                                  e.strerror))
+
         if self.src == '' or self.src[-1] != '\n':
             self.src += '\n'
         self.cursor = 0
-        self.info = QAPISourceInfo(self.fname, 1, incl_info)
+        self.info = QAPISourceInfo(fname, 1, incl_info)
         self.line_pos = 0
         self.exprs = []
         self.docs = []
@@ -413,7 +430,7 @@  class QAPISchemaParser(object):
                 if not isinstance(include, str):
                     raise QAPISemError(info,
                                        "value of 'include' must be a string")
-                incl_fname = os.path.join(os.path.dirname(self.fname),
+                incl_fname = os.path.join(os.path.dirname(fname),
                                           include)
                 self.exprs.append({'expr': {'include': incl_fname},
                                    'info': info})
@@ -465,14 +482,7 @@  class QAPISchemaParser(object):
         if incl_abs_fname in previously_included:
             return None
 
-        try:
-            if sys.version_info[0] >= 3:
-                fobj = open(incl_fname, 'r', encoding='utf-8')
-            else:
-                fobj = open(incl_fname, 'r')
-        except IOError as e:
-            raise QAPISemError(info, "%s: %s" % (e.strerror, incl_fname))
-        return QAPISchemaParser(fobj, previously_included, info)
+        return QAPISchemaParser(incl_fname, previously_included, info)
 
     def _pragma(self, name, value, info):
         global doc_required, returns_whitelist, name_case_whitelist
@@ -1723,11 +1733,7 @@  class QAPISchemaEvent(QAPISchemaEntity):
 class QAPISchema(object):
     def __init__(self, fname):
         self.fname = fname
-        if sys.version_info[0] >= 3:
-            f = open(fname, 'r', encoding='utf-8')
-        else:
-            f = open(fname, 'r')
-        parser = QAPISchemaParser(f)
+        parser = QAPISchemaParser(fname)
         exprs = check_exprs(parser.exprs)
         self.docs = parser.docs
         self._entity_list = []
diff --git a/tests/qapi-schema/include-no-file.err b/tests/qapi-schema/include-no-file.err
index e42bcf4bc1..0a6c6bb4a9 100644
--- a/tests/qapi-schema/include-no-file.err
+++ b/tests/qapi-schema/include-no-file.err
@@ -1 +1 @@ 
-tests/qapi-schema/include-no-file.json:1: No such file or directory: tests/qapi-schema/include-no-file-sub.json
+tests/qapi-schema/include-no-file.json:1: can't read include file 'tests/qapi-schema/include-no-file-sub.json': No such file or directory