qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: den@openvz.org
Subject: Re: [Qemu-devel] [PATCH 2/2] nbd/server: add assert to nbd_negotiate_handle_info
Date: Thu, 2 Nov 2017 13:06:35 -0500	[thread overview]
Message-ID: <0f8feedf-9650-f004-dae0-c658759588d9@redhat.com> (raw)
In-Reply-To: <20171101154204.27146-2-vsementsov@virtuozzo.com>

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

On 11/01/2017 10:42 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add an assert here to make last length assignment meaningful and
> following return without tail dropping obvious.

Not quite sure I followed your explanation, which means it's difficult
for me to propose an alternative wording.  Maybe:

Add an assert here to make it obvious that the prior loop consumed the
rest of the input, and that all further code in the function is focused
on output.

On the other hand, if you are okay with it, I wouldn't mind squashing
the first and second patches into one (as the first patch is then easier
to read when it is obvious that we used the wrong length variable).  But
until I get your feedback (on either squashing the two patches or
tweaking the wording), I'm just placing your patches as-is on my NBD
queue for inclusion prior to rc0.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  nbd/server.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

  reply	other threads:[~2017-11-02 18:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-01 15:42 [Qemu-devel] [PATCH 1/2] nbd/server: fix nbd_negotiate_handle_info Vladimir Sementsov-Ogievskiy
2017-11-01 15:42 ` [Qemu-devel] [PATCH 2/2] nbd/server: add assert to nbd_negotiate_handle_info Vladimir Sementsov-Ogievskiy
2017-11-02 18:06   ` Eric Blake [this message]
2017-11-03  5:50     ` Vladimir Sementsov-Ogievskiy
2017-11-02 17:54 ` [Qemu-devel] [PATCH 1/2] nbd/server: fix nbd_negotiate_handle_info Eric Blake
2017-11-03  5:41   ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0f8feedf-9650-f004-dae0-c658759588d9@redhat.com \
    --to=eblake@redhat.com \
    --cc=den@openvz.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).