[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/17] nbd/client: refactor nbd_read_eof
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 02/17] nbd/client: refactor nbd_read_eof |
Date: |
Fri, 25 Aug 2017 14:22:06 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
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 <address@hidden>
>>> ---
>>> + * 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 <address@hidden>
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
- Re: [Qemu-devel] [PATCH 05/17] block/nbd-client: get rid of ssize_t, (continued)
[Qemu-devel] [PATCH 01/17] nbd/client: fix nbd_opt_go, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-devel] [PATCH 03/17] nbd/client: refactor nbd_receive_reply, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-devel] [PATCH 08/17] block/nbd-client: rename nbd_recv_coroutines_enter_all, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-devel] [PATCH 02/17] nbd/client: refactor nbd_read_eof, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-devel] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Vladimir Sementsov-Ogievskiy, 2017/08/04
- Re: [Qemu-devel] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Eric Blake, 2017/08/07
- Re: [Qemu-devel] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Vladimir Sementsov-Ogievskiy, 2017/08/07
- Re: [Qemu-devel] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Eric Blake, 2017/08/07
- Re: [Qemu-devel] [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Eric Blake, 2017/08/07
- Re: [Qemu-devel] [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Vladimir Sementsov-Ogievskiy, 2017/08/07
- Re: [Qemu-devel] [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Eric Blake, 2017/08/07
[Qemu-devel] [PATCH 14/17] block/nbd-client: exit reply-reading coroutine on incorrect handle, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-devel] [PATCH 17/17] block/nbd-client: always return EIO on and after the first io channel error, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-devel] [PATCH 09/17] block/nbd-client: move nbd_co_receive_reply content into nbd_co_request, Vladimir Sementsov-Ogievskiy, 2017/08/04