| Message ID | 1492957997-28587-2-git-send-email-lidongchen@tencent.com |
|---|---|
| State | New |
| Headers | show |
On 04/23/2017 09:33 AM, jemmy858585@gmail.com wrote: > From: Lidong Chen <lidongchen@tencent.com> > > Fix some spelling errors in is_allocated_sectors comment. > > Signed-off-by: Lidong Chen <lidongchen@tencent.com> > --- > qemu-img.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/qemu-img.c b/qemu-img.c > index df6d165..0b3349c 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1033,8 +1033,8 @@ done: > } > > /* > - * Returns true iff the first sector pointed to by 'buf' contains at least > - * a non-NUL byte. > + * Returns true if the first sector pointed to by 'buf' contains at least > + * a non-NULL byte. NACK to both changes. 'iff' is an English word that is shorthand for "if and only if". "NUL" means the one-byte character, while "NULL" means the 8-byte (or 4-byte, on 32-bit platform) pointer value.
Hi Eric, On 04/24/2017 11:40 AM, Eric Blake wrote: > On 04/23/2017 09:33 AM, jemmy858585@gmail.com wrote: >> From: Lidong Chen <lidongchen@tencent.com> >> >> Fix some spelling errors in is_allocated_sectors comment. >> >> Signed-off-by: Lidong Chen <lidongchen@tencent.com> >> --- >> qemu-img.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index df6d165..0b3349c 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c >> @@ -1033,8 +1033,8 @@ done: >> } >> >> /* >> - * Returns true iff the first sector pointed to by 'buf' contains at least >> - * a non-NUL byte. >> + * Returns true if the first sector pointed to by 'buf' contains at least >> + * a non-NULL byte. > > NACK to both changes. 'iff' is an English word that is shorthand for > "if and only if". "NUL" means the one-byte character, while "NULL" > means the 8-byte (or 4-byte, on 32-bit platform) pointer value. I agree with Lidong shorthands are not obvious from non-native speaker. What about this? * Returns true if (and only if) the first sector pointed to by 'buf' contains * at least a non-null character.
On 04/24/2017 10:37 AM, Philippe Mathieu-Daudé wrote: >>> /* >>> - * Returns true iff the first sector pointed to by 'buf' contains at >>> least >>> - * a non-NUL byte. >>> + * Returns true if the first sector pointed to by 'buf' contains at >>> least >>> + * a non-NULL byte. >> >> NACK to both changes. 'iff' is an English word that is shorthand for >> "if and only if". "NUL" means the one-byte character, while "NULL" >> means the 8-byte (or 4-byte, on 32-bit platform) pointer value. > > I agree with Lidong shorthands are not obvious from non-native speaker. > > What about this? > > * Returns true if (and only if) the first sector pointed to by 'buf' > contains That might be okay. > * at least a non-null character. But that still doesn't make sense. The character name is NUL, and non-NULL refers to something that is a pointer, not a character.
On 04/24/2017 10:47 AM, Eric Blake wrote: > On 04/24/2017 10:37 AM, Philippe Mathieu-Daudé wrote: > >>>> /* >>>> - * Returns true iff the first sector pointed to by 'buf' contains at >>>> least >>>> - * a non-NUL byte. >>>> + * Returns true if the first sector pointed to by 'buf' contains at >>>> least >>>> + * a non-NULL byte. >>> >>> NACK to both changes. 'iff' is an English word that is shorthand for >>> "if and only if". "NUL" means the one-byte character, while "NULL" >>> means the 8-byte (or 4-byte, on 32-bit platform) pointer value. >> >> I agree with Lidong shorthands are not obvious from non-native speaker. >> >> What about this? >> >> * Returns true if (and only if) the first sector pointed to by 'buf' >> contains > > That might be okay. > >> * at least a non-null character. > > But that still doesn't make sense. The character name is NUL, and > non-NULL refers to something that is a pointer, not a character. What's more, the NUL character can actually occupy more than one byte (think UTF-16, where it is the two-byte 0 value). Referring to NUL byte rather than NUL character (or even the 'zero byte') makes it obvious that this function is NOT encoding-sensitive, and doesn't start mis-behaving just because the data picks a multi-byte character encoding.
On Mon, Apr 24, 2017 at 11:53 PM, Eric Blake <eblake@redhat.com> wrote: > On 04/24/2017 10:47 AM, Eric Blake wrote: >> On 04/24/2017 10:37 AM, Philippe Mathieu-Daudé wrote: >> >>>>> /* >>>>> - * Returns true iff the first sector pointed to by 'buf' contains at >>>>> least >>>>> - * a non-NUL byte. >>>>> + * Returns true if the first sector pointed to by 'buf' contains at >>>>> least >>>>> + * a non-NULL byte. >>>> >>>> NACK to both changes. 'iff' is an English word that is shorthand for >>>> "if and only if". "NUL" means the one-byte character, while "NULL" >>>> means the 8-byte (or 4-byte, on 32-bit platform) pointer value. >>> >>> I agree with Lidong shorthands are not obvious from non-native speaker. >>> >>> What about this? >>> >>> * Returns true if (and only if) the first sector pointed to by 'buf' >>> contains >> >> That might be okay. >> >>> * at least a non-null character. >> >> But that still doesn't make sense. The character name is NUL, and >> non-NULL refers to something that is a pointer, not a character. > > What's more, the NUL character can actually occupy more than one byte > (think UTF-16, where it is the two-byte 0 value). Referring to NUL byte > rather than NUL character (or even the ' byte') makes it obvious > that this function is NOT encoding-sensitive, and doesn't start > mis-behaving just because the data picks a multi-byte character encoding. How about this? * Returns true if (and only if) the first sector pointed to by 'buf' contains at least * a non-zero byte. Thanks. > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org >
On 24.04.2017 17:53, Eric Blake wrote: > On 04/24/2017 10:47 AM, Eric Blake wrote: >> On 04/24/2017 10:37 AM, Philippe Mathieu-Daudé wrote: >> >>>>> /* >>>>> - * Returns true iff the first sector pointed to by 'buf' contains at >>>>> least >>>>> - * a non-NUL byte. >>>>> + * Returns true if the first sector pointed to by 'buf' contains at >>>>> least >>>>> + * a non-NULL byte. >>>> >>>> NACK to both changes. 'iff' is an English word that is shorthand for >>>> "if and only if". "NUL" means the one-byte character, while "NULL" >>>> means the 8-byte (or 4-byte, on 32-bit platform) pointer value. >>> >>> I agree with Lidong shorthands are not obvious from non-native speaker. >>> >>> What about this? >>> >>> * Returns true if (and only if) the first sector pointed to by 'buf' >>> contains >> >> That might be okay. Might, yes, but we have it all over the code. I'm not particularly avid to change this, because I am in fact one of the culprits (and I'm a non-native speaker, but I do like to use LaTeX so I know my \iff). (By the way, judging from the author's name of this line of code (which is Thiemo Seufer), I'd wager he's not a native speaker either.) >>> * at least a non-null character. >> >> But that still doesn't make sense. The character name is NUL, and >> non-NULL refers to something that is a pointer, not a character. > > What's more, the NUL character can actually occupy more than one byte > (think UTF-16, where it is the two-byte 0 value). Referring to NUL byte > rather than NUL character (or even the 'zero byte') makes it obvious > that this function is NOT encoding-sensitive, and doesn't start > mis-behaving just because the data picks a multi-byte character encoding. Furthermore, this doesn't have anything to do with being a native speaker or not: NUL is just the commonly used and probably standardized abbreviation of a certain ASCII character (in any language). It's OK not to know this, but I don't think it's OK to change the comment. Max
On Wed, Apr 26, 2017 at 3:11 AM, Max Reitz <mreitz@redhat.com> wrote: > On 24.04.2017 17:53, Eric Blake wrote: >> On 04/24/2017 10:47 AM, Eric Blake wrote: >>> On 04/24/2017 10:37 AM, Philippe Mathieu-Daudé wrote: >>> >>>>>> /* >>>>>> - * Returns true iff the first sector pointed to by 'buf' contains at >>>>>> least >>>>>> - * a non-NUL byte. >>>>>> + * Returns true if the first sector pointed to by 'buf' contains at >>>>>> least >>>>>> + * a non-NULL byte. >>>>> >>>>> NACK to both changes. 'iff' is an English word that is shorthand for >>>>> "if and only if". "NUL" means the one-byte character, while "NULL" >>>>> means the 8-byte (or 4-byte, on 32-bit platform) pointer value. >>>> >>>> I agree with Lidong shorthands are not obvious from non-native speaker. >>>> >>>> What about this? >>>> >>>> * Returns true if (and only if) the first sector pointed to by 'buf' >>>> contains >>> >>> That might be okay. > > Might, yes, but we have it all over the code. I'm not particularly avid > to change this, because I am in fact one of the culprits (and I'm a > non-native speaker, but I do like to use LaTeX so I know my \iff). > > (By the way, judging from the author's name of this line of code (which > is Thiemo Seufer), I'd wager he's not a native speaker either.) > >>>> * at least a non-null character. >>> >>> But that still doesn't make sense. The character name is NUL, and >>> non-NULL refers to something that is a pointer, not a character. >> >> What's more, the NUL character can actually occupy more than one byte >> (think UTF-16, where it is the two-byte 0 value). Referring to NUL byte >> rather than NUL character (or even the 'zero byte') makes it obvious >> that this function is NOT encoding-sensitive, and doesn't start >> mis-behaving just because the data picks a multi-byte character encoding. > > Furthermore, this doesn't have anything to do with being a native > speaker or not: NUL is just the commonly used and probably standardized > abbreviation of a certain ASCII character (in any language). It's OK not > to know this, but I don't think it's OK to change the comment. Thanks for your explanation. > > Max >
diff --git a/qemu-img.c b/qemu-img.c index df6d165..0b3349c 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1033,8 +1033,8 @@ done: } /* - * Returns true iff the first sector pointed to by 'buf' contains at least - * a non-NUL byte. + * Returns true if the first sector pointed to by 'buf' contains at least + * a non-NULL byte. * * 'pnum' is set to the number of sectors (including and immediately following * the first one) that are known to be in the same allocated/unallocated state.