qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: kwolf@redhat.com, lcapitulino@redhat.com, qemu-devel@nongnu.org,
	mdroth@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH V7 03/11] qapi-script: remember line number in schema parsing
Date: Fri, 21 Feb 2014 08:10:20 +0800	[thread overview]
Message-ID: <5306996C.2000709@linux.vnet.ibm.com> (raw)
In-Reply-To: <87eh2y0y9r.fsf@blackfin.pond.sub.org>

于 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.

  reply	other threads:[~2014-02-21  0:10 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-20  5:54 [Qemu-devel] [PATCH V7 00/11] qapi script: support enum as discriminator and better enum name Wenchao Xia
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 01/11] qapi script: remember enum values Wenchao Xia
2014-02-20 12:05   ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 02/11] qapi script: add check for duplicated key Wenchao Xia
2014-02-20 12:05   ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 03/11] qapi-script: remember line number in schema parsing Wenchao Xia
2014-02-20 12:22   ` Markus Armbruster
2014-02-21  0:10     ` Wenchao Xia [this message]
2014-02-21 13:04       ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 04/11] qapi script: check correctness of discriminator values in union Wenchao Xia
2014-02-20 14:43   ` Markus Armbruster
2014-02-20 15:26     ` Eric Blake
2014-02-21  8:21       ` Markus Armbruster
2014-02-21 13:49         ` Eric Blake
2014-02-21 14:08           ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 05/11] qapi script: code move for generate_enum_name() Wenchao Xia
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 06/11] qapi script: use same function to generate enum string Wenchao Xia
2014-02-20 15:20   ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 07/11] qapi script: support pre-defined enum type as discriminator in union Wenchao Xia
2014-02-20 16:38   ` Markus Armbruster
2014-02-21  0:17     ` Wenchao Xia
2014-02-21  8:13       ` Markus Armbruster
2014-02-21 13:56         ` Eric Blake
2014-02-21 14:39           ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 08/11] qapi: convert BlockdevOptions to use enum discriminator Wenchao Xia
2014-02-20 17:59   ` Eric Blake
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 09/11] qapi script: do not allow string discriminator Wenchao Xia
2014-02-20 16:50   ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 10/11] qapi script: do not add "_" for every capitalized char in enum Wenchao Xia
2014-02-20 16:54   ` Markus Armbruster
2014-02-20 17:53     ` Eric Blake
2014-02-21  8:21       ` Markus Armbruster
2014-02-20  5:54 ` [Qemu-devel] [PATCH V7 11/11] qapi test: add error path test for union Wenchao Xia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5306996C.2000709@linux.vnet.ibm.com \
    --to=xiawenc@linux.vnet.ibm.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).