On 08/07/2017 07:05 AM, Vladimir Sementsov-Ogievskiy wrote: > 07.08.2017 14:42, Eric Blake wrote: >> On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: >>> Refactor nbd_read_eof to return 1 on success, 0 on eof, when no >>> data was read and <0 for other cases, because returned size of >>> read data is not actually used. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >>> ---
>>> + * Returns 1 on success
>>> + * 0 on eof, when no data was read (errp is not set)
>>> + * -EINVAL on eof, when some data < @size was read until eof
>>> + * < 0 on read fail
>> In general, mixing negative errno value and generic < 0 in the same
>> function is most likely ambiguous.
>
> Hmm, but this is entirely what we do so often:
>
> if (,,) return -EINVAL;
>
> return some_other_func().
>
> last two lines may be rewritten like this:
> + * < 0 on fail
Or even better as 'negative errno on failure'.
Here's what I'm proposing to squash in, at which point you have:
Reviewed-by: Eric Blake <ebl...@redhat.com>
diff --git i/nbd/nbd-internal.h w/nbd/nbd-internal.h
index 3fb0b6098a..03549e3f39 100644
--- i/nbd/nbd-internal.h
+++ w/nbd/nbd-internal.h
@@ -80,8 +80,7 @@
* Tries to read @size bytes from @ioc.
* Returns 1 on success
* 0 on eof, when no data was read (errp is not set)
- * -EINVAL on eof, when some data < @size was read until eof
- * < 0 on read fail
+ * negative errno on failure (errp is set)
*/
static inline int nbd_read_eof(QIOChannel *ioc, void *buffer, size_t size,
Error **errp)
@@ -95,7 +94,7 @@ static inline int nbd_read_eof(QIOChannel *ioc, void
*buffer, size_t size,
* that this is coroutine-safe.
*/
- assert(size > 0);
+ assert(size);
ret = nbd_rwv(ioc, &iov, 1, size, true, errp);
if (ret <= 0) {
>>> +
>>> + if (ret != size) {
>>> + error_setg(errp, "End of file");
>>> + return -EINVAL;
>> and so is this. Which makes the function documentation not quite
>> accurate; you aren't mixing a generic < 0.
>
> hmm.. my wordings are weird sometimes, sorry for that :(. and thank you
> for your patience.
Not a problem - I understand that English is not your native language,
so you are already a leg up on me (I'm nowhere near as competent in a
second language as you already are on English, even if review still
gives you grammar hints and other improvements).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
