Message ID | 1392875695-15627-4-git-send-email-xiawenc@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > Before this patch, 'QAPISchemaError' scans whole input until 'pos' > to get error line number. After this patch, the scan is avoided since > line number is remembered in schema parsing. This patch also benefits > other error report functions, which would be introduced later. Not sure avoiding the scan is worthwhile, but since you coded it already... no objections. > > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > scripts/qapi.py | 14 ++++++++------ > 1 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 3732fe1..c504eb4 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -39,12 +39,10 @@ class QAPISchemaError(Exception): > def __init__(self, schema, msg): > self.fp = schema.fp > self.msg = msg > - self.line = self.col = 1 > - for ch in schema.src[0:schema.pos]: > - if ch == '\n': > - self.line += 1 > - self.col = 1 > - elif ch == '\t': > + self.col = 1 > + self.line = schema.line > + for ch in schema.src[schema.line_pos:schema.pos]: > + if ch == '\t': > self.col = (self.col + 7) % 8 + 1 Column computation is wrong. Should be something like self.col = ((self.col + 7) & ~7) + 1 Not your fault, of course, and you don't have to fix it to get my R-by. If you want to fix it, separate patch, and please include suitable tests. > else: > self.col += 1 > @@ -60,6 +58,8 @@ class QAPISchema: > if self.src == '' or self.src[-1] != '\n': > self.src += '\n' > self.cursor = 0 > + self.line = 1 > + self.line_pos = 0 > self.exprs = [] > self.accept() > > @@ -100,6 +100,8 @@ class QAPISchema: > if self.cursor == len(self.src): > self.tok = None > return > + self.line += 1 > + self.line_pos = self.cursor > elif not self.tok.isspace(): > raise QAPISchemaError(self, 'Stray "%s"' % self.tok)
δΊ 2014/2/20 20:22, Markus Armbruster ει: > Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > >> Before this patch, 'QAPISchemaError' scans whole input until 'pos' >> to get error line number. After this patch, the scan is avoided since >> line number is remembered in schema parsing. This patch also benefits >> other error report functions, which would be introduced later. > > Not sure avoiding the scan is worthwhile, but since you coded it > already... no objections. > >> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >> --- >> scripts/qapi.py | 14 ++++++++------ >> 1 files changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/scripts/qapi.py b/scripts/qapi.py >> index 3732fe1..c504eb4 100644 >> --- a/scripts/qapi.py >> +++ b/scripts/qapi.py >> @@ -39,12 +39,10 @@ class QAPISchemaError(Exception): >> def __init__(self, schema, msg): >> self.fp = schema.fp >> self.msg = msg >> - self.line = self.col = 1 >> - for ch in schema.src[0:schema.pos]: >> - if ch == '\n': >> - self.line += 1 >> - self.col = 1 >> - elif ch == '\t': >> + self.col = 1 >> + self.line = schema.line >> + for ch in schema.src[schema.line_pos:schema.pos]: >> + if ch == '\t': >> self.col = (self.col + 7) % 8 + 1 > > Column computation is wrong. Should be something like > > self.col = ((self.col + 7) & ~7) + 1 > > Not your fault, of course, and you don't have to fix it to get my R-by. > If you want to fix it, separate patch, and please include suitable > tests. > Thanks for your quick review, I'll respin it next week.
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> Thanks for your quick review, I'll respin it next week.
Looking forward to it. I think we're very close to merge.
diff --git a/scripts/qapi.py b/scripts/qapi.py index 3732fe1..c504eb4 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -39,12 +39,10 @@ class QAPISchemaError(Exception): def __init__(self, schema, msg): self.fp = schema.fp self.msg = msg - self.line = self.col = 1 - for ch in schema.src[0:schema.pos]: - if ch == '\n': - self.line += 1 - self.col = 1 - elif ch == '\t': + self.col = 1 + self.line = schema.line + for ch in schema.src[schema.line_pos:schema.pos]: + if ch == '\t': self.col = (self.col + 7) % 8 + 1 else: self.col += 1 @@ -60,6 +58,8 @@ class QAPISchema: if self.src == '' or self.src[-1] != '\n': self.src += '\n' self.cursor = 0 + self.line = 1 + self.line_pos = 0 self.exprs = [] self.accept() @@ -100,6 +100,8 @@ class QAPISchema: if self.cursor == len(self.src): self.tok = None return + self.line += 1 + self.line_pos = self.cursor elif not self.tok.isspace(): raise QAPISchemaError(self, 'Stray "%s"' % self.tok)
Before this patch, 'QAPISchemaError' scans whole input until 'pos' to get error line number. After this patch, the scan is avoided since line number is remembered in schema parsing. This patch also benefits other error report functions, which would be introduced later. Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- scripts/qapi.py | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)