Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/3] qemu-io: Correct error messages

2015-10-27 Thread Eric Blake
On 10/27/2015 09:50 AM, John Snow wrote:

>>> +default:
>>> +printf("Parsing error -- %s\n", arg);
>>
>> I still think ':' is better than ' --' in error messages, but I'll leave
>> it up to the maintainer.
>>
> 
> Crud, sorry Eric -- I didn't do this on purpose. As Kevin notes, I was
> just trying to match the existing format. I can change it and send again
> if you want. Whatever is easiest for people.

And Kevin has a valid point that you just did code motion, so keeping --
is no worse than before.  At this point, I'll leave it up to you; my R-b
stands either way.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/3] qemu-io: Correct error messages

2015-10-27 Thread Kevin Wolf
Am 27.10.2015 um 16:50 hat John Snow geschrieben:
> 
> 
> On 10/26/2015 10:26 PM, Eric Blake wrote:
> > On 10/26/2015 05:45 PM, John Snow wrote:
> >> Reported-by: Max Reitz 
> >> Signed-off-by: John Snow 
> >> Reviewed-by: Eric Blake 
> >> ---
> >>  qemu-io-cmds.c | 53 ++---
> >>  1 file changed, 34 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> >> index 44d24e8..92c6b87 100644
> >> --- a/qemu-io-cmds.c
> >> +++ b/qemu-io-cmds.c
> >> @@ -146,6 +146,21 @@ static int64_t cvtnum(const char *s)
> >>  return ret;
> >>  }
> >>  
> >> +static void print_cvtnum_err(int64_t rc, const char *arg)
> >> +{
> >> +switch (rc) {
> >> +case -EINVAL:
> >> +printf("Parsing error: non-numeric argument,"
> >> +   " or extraneous/unrecognized suffix -- %s\n", arg);
> >> +break;
> >> +case -ERANGE:
> >> +printf("Parsing error: argument too large -- %s\n", arg);
> >> +break;
> >> +default:
> >> +printf("Parsing error -- %s\n", arg);
> > 
> > I still think ':' is better than ' --' in error messages, but I'll leave
> > it up to the maintainer.
> 
> Crud, sorry Eric -- I didn't do this on purpose. As Kevin notes, I was
> just trying to match the existing format. I can change it and send again
> if you want. Whatever is easiest for people.

I think you need to respin for patch 1 anyway, so changing it in the
next version sounds good. You can keep my R-b when doing this.

Kevin



Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/3] qemu-io: Correct error messages

2015-10-27 Thread John Snow


On 10/26/2015 10:26 PM, Eric Blake wrote:
> On 10/26/2015 05:45 PM, John Snow wrote:
>> Reported-by: Max Reitz 
>> Signed-off-by: John Snow 
>> Reviewed-by: Eric Blake 
>> ---
>>  qemu-io-cmds.c | 53 ++---
>>  1 file changed, 34 insertions(+), 19 deletions(-)
>>
>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>> index 44d24e8..92c6b87 100644
>> --- a/qemu-io-cmds.c
>> +++ b/qemu-io-cmds.c
>> @@ -146,6 +146,21 @@ static int64_t cvtnum(const char *s)
>>  return ret;
>>  }
>>  
>> +static void print_cvtnum_err(int64_t rc, const char *arg)
>> +{
>> +switch (rc) {
>> +case -EINVAL:
>> +printf("Parsing error: non-numeric argument,"
>> +   " or extraneous/unrecognized suffix -- %s\n", arg);
>> +break;
>> +case -ERANGE:
>> +printf("Parsing error: argument too large -- %s\n", arg);
>> +break;
>> +default:
>> +printf("Parsing error -- %s\n", arg);
> 
> I still think ':' is better than ' --' in error messages, but I'll leave
> it up to the maintainer.
> 

Crud, sorry Eric -- I didn't do this on purpose. As Kevin notes, I was
just trying to match the existing format. I can change it and send again
if you want. Whatever is easiest for people.

--js