From patchwork Fri Apr 22 23:40:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 8916551 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 9EFFBBF29F for ; Sat, 23 Apr 2016 00:01:23 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D54832020F for ; Sat, 23 Apr 2016 00:01:22 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E0E4E201C7 for ; Sat, 23 Apr 2016 00:01:21 +0000 (UTC) Received: from localhost ([::1]:41992 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atl0j-0004DB-Aw for patchwork-qemu-devel@patchwork.kernel.org; Fri, 22 Apr 2016 20:01:21 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45535) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atkhm-0004Bm-V2 for qemu-devel@nongnu.org; Fri, 22 Apr 2016 19:41:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1atkhl-0005WW-LJ for qemu-devel@nongnu.org; Fri, 22 Apr 2016 19:41:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54421) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atkhi-0005Ty-44; Fri, 22 Apr 2016 19:41:42 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B2D8783F45; Fri, 22 Apr 2016 23:41:41 +0000 (UTC) Received: from red.redhat.com (ovpn-113-21.phx2.redhat.com [10.3.113.21]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3MNfHXN028475; Fri, 22 Apr 2016 19:41:41 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 22 Apr 2016 17:40:42 -0600 Message-Id: <1461368452-10389-35-git-send-email-eblake@redhat.com> In-Reply-To: <1461368452-10389-1-git-send-email-eblake@redhat.com> References: <1461368452-10389-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v3 34/44] nbd: Less allocation during NBD_OPT_LIST X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , alex@alex.org.uk, qemu-block@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Since we know that the maximum name we are willing to accept is small enough to stack-allocate, rework the iteration over NBD_OPT_LIST responses to reuse a stack buffer rather than allocating every time. Furthermore, we don't even have to allocate if we know the server's length doesn't match what we are searching for. Signed-off-by: Eric Blake Reviewed-by: Alex Bligh --- v3: tweak commit message --- nbd/client.c | 130 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index d346c86..4debb5d 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -230,14 +230,17 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply, return result; } -static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp) +/* Return -1 if unrecoverable error occurs, 0 if NBD_OPT_LIST is + * unsupported, 1 if iteration is done, 2 to keep looking, and 3 if + * this entry matches want. */ +static int nbd_receive_list(QIOChannel *ioc, const char *want, Error **errp) { nbd_opt_reply reply; uint32_t len; uint32_t namelen; + char name[NBD_MAX_NAME_SIZE + 1]; int error; - *name = NULL; if (nbd_receive_option_reply(ioc, NBD_OPT_LIST, &reply, errp) < 0) { return -1; } @@ -252,97 +255,94 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp) error_setg(errp, "length too long for option end"); return -1; } - } else if (reply.type == NBD_REP_SERVER) { - if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) { - error_setg(errp, "incorrect option length %"PRIu32, len); - return -1; - } - if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) { - error_setg(errp, "failed to read option name length"); - return -1; - } - namelen = be32_to_cpu(namelen); - len -= sizeof(namelen); - if (len < namelen) { - error_setg(errp, "incorrect option name length"); - return -1; - } - if (namelen > NBD_MAX_NAME_SIZE) { - error_setg(errp, "export name length too long %" PRIu32, namelen); - return -1; - } - - *name = g_new0(char, namelen + 1); - if (read_sync(ioc, *name, namelen) != namelen) { - error_setg(errp, "failed to read export name"); - g_free(*name); - *name = NULL; - return -1; - } - (*name)[namelen] = '\0'; - len -= namelen; - if (drop_sync(ioc, len) != len) { - error_setg(errp, "failed to read export description"); - g_free(*name); - *name = NULL; - return -1; - } - } else { + return 1; + } else if (reply.type != NBD_REP_SERVER) { error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x", reply.type, NBD_REP_SERVER); return -1; } - return 1; + + if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) { + error_setg(errp, "incorrect option length %"PRIu32, len); + return -1; + } + if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) { + error_setg(errp, "failed to read option name length"); + return -1; + } + namelen = be32_to_cpu(namelen); + len -= sizeof(namelen); + if (len < namelen) { + error_setg(errp, "incorrect option name length"); + return -1; + } + if (namelen != strlen(want)) { + if (drop_sync(ioc, len) != len) { + error_setg(errp, "failed to skip export name with wrong length"); + return -1; + } + return 2; + } + + assert(namelen < sizeof(name)); + if (read_sync(ioc, name, namelen) != namelen) { + error_setg(errp, "failed to read export name"); + return -1; + } + name[namelen] = '\0'; + len -= namelen; + if (drop_sync(ioc, len) != len) { + error_setg(errp, "failed to read export description"); + return -1; + } + return strcmp(name, want) == 0 ? 3 : 2; } +/* Return -1 on failure, 0 if wantname is an available export. */ static int nbd_receive_query_exports(QIOChannel *ioc, const char *wantname, Error **errp) { bool foundExport = false; - TRACE("Querying export list"); + TRACE("Querying export list for '%s'", wantname); if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) { return -1; } TRACE("Reading available export names"); while (1) { - char *name = NULL; - int ret = nbd_receive_list(ioc, &name, errp); + int ret = nbd_receive_list(ioc, wantname, errp); - if (ret < 0) { - g_free(name); - name = NULL; + switch (ret) { + default: + /* Server gave unexpected reply */ + assert(ret < 0); return -1; - } - if (ret == 0) { + case 0: /* Server doesn't support export listing, so * we will just assume an export with our * wanted name exists */ - foundExport = true; - break; - } - if (name == NULL) { - TRACE("End of export name list"); + return 0; + case 1: + /* Done iterating. */ + if (!foundExport) { + error_setg(errp, "No export with name '%s' available", + wantname); + return -1; + } + return 0; + case 2: + /* Wasn't this one, keep going. */ break; - } - if (g_str_equal(name, wantname)) { + case 3: + /* Found a match, but must finish parsing reply. */ + TRACE("Found desired export name '%s'", wantname); foundExport = true; - TRACE("Found desired export name '%s'", name); - } else { - TRACE("Ignored export name '%s'", name); + break; } - g_free(name); - } - - if (!foundExport) { - error_setg(errp, "No export with name '%s' available", wantname); - return -1; } - - return 0; } static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,