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 <ebl...@redhat.com> >>> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> >>> > >>> } >>> + 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
signature.asc
Description: OpenPGP digital signature