qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v10 07/17] qemu-io: Switch 'alloc' command to by


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v10 07/17] qemu-io: Switch 'alloc' command to byte-based length
Date: Fri, 28 Apr 2017 22:09:10 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0

On 28.04.2017 21:59, Eric Blake wrote:
> On 04/28/2017 02:46 PM, Max Reitz wrote:
>> On 27.04.2017 03:46, Eric Blake wrote:
>>> For the 'alloc' command, accepting an offset in bytes but a length
>>> in sectors, and reporting output in sectors, is confusing.  Do
>>> everything in bytes, and adjust the expected output accordingly.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>>>
> 
>>>      }
>>> +    if (!QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)) {
>>> +        printf("bytes %" PRId64 " is not sector aligned\n",
>>
>> This isn't real English. :-)
> 
> But, it's just copy-and-paste from the other instances you just reviewed
> in 6/17!  [Translation - if I change this one, I also get to redo that one]

No, those are "offset" and "count" -- all singular. "bytes" is plural. ;-)

> 
> Which of these various alternatives (if any) looks better:
> 
> bytes=511 is not sector-aligned
> 511 is not a sector-aligned value for 'bytes'
> requested 'bytes' of 511 is not sector-aligned
> alignment error: 511 bytes is not sector-aligned> 'bytes' must be 
> sector-aligned: 511
> your clever entry here...

How about "byte count" instead of "bytes" or "bytes value", if really
want to have the exact spelling in there?

For your entries above: (1) and (2) work for me (I like (2) a bit
better), (3) doesn't sound like real English either, and it should be
s/is/are/ in (4) (but it still sounds off with that change). (5) I
mostly dislike because I dislike error message of the form "This should
be X: $foo", I like "$foo is not X" better.

>> With that fixed (somehow, you know better than me how to):
> 
> Re-reading my various alternatives, I do think that /sector
> aligned/sector-aligned/ helps no matter what; and that the remaining
> trick is to use quoting or '=' or some other lexical trick to make it
> obvious that 'bytes' is a parameter name whose value 511 is invalid,
> rather than part of the actual error of a value that is not properly
> aligned.

First of all, I think the user is clever enough to figure out that a
description like "byte count" refers to the "bytes" parameter. ;-)

Secondly, this is even more true because this is a debugging tool so we
don't have to worry too much about... Let's say inexperienced users.

Max

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]