From patchwork Wed Apr 27 17:01:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 8960671 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 259389F457 for ; Wed, 27 Apr 2016 17:04:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2F7DF201EC for ; Wed, 27 Apr 2016 17:04:03 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3FEDC2022D for ; Wed, 27 Apr 2016 17:04:02 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1avSqe-0008Se-55; Wed, 27 Apr 2016 17:02:00 +0000 Received: from mail6.bemta6.messagelabs.com ([85.158.143.247]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1avSqc-0008P9-Q0 for xen-devel@lists.xen.org; Wed, 27 Apr 2016 17:01:58 +0000 Received: from [85.158.143.35] by server-1.bemta-6.messagelabs.com id 8B/34-18833-680F0275; Wed, 27 Apr 2016 17:01:58 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprAIsWRWlGSWpSXmKPExsXitHRDpG7bB4V wgxf/2C2WfFzM4sDocXT3b6YAxijWzLyk/IoE1ozWm9vYCrYpVXw+dJW9gXGPTBcjJ4eEgL/E /F9HWUBsNgF9id0vPjGB2CIC6hKnOy6ygtjMAmUSUye9ZgaxhQVSJW6sPsQIYrMIqEosvXmeH cTmFfCUWPa/mx1ippzE+eM/weo5BbwkXj05AhYXAqqZcO8+E4StJnGt/xJUr6DEyZlPWCB2SU gcfPGCGWIOt8Tt01OZJzDyzUJSNgtJ2QJGplWM6sWpRWWpRbrGeklFmekZJbmJmTm6hgZmerm pxcWJ6ak5iUnFesn5uZsYgSHFAAQ7GDv+OR1ilORgUhLlXXJWIVyILyk/pTIjsTgjvqg0J7X4 EKMMB4eSBO/t90A5waLU9NSKtMwcYHDDpCU4eJREeEvfAaV5iwsSc4sz0yFSpxgVpcR5N4P0C YAkMkrz4NpgEXWJUVZKmJcR6BAhnoLUotzMElT5V4ziHIxKwrxbQabwZOaVwE1/BbSYCWjx5U OyIItLEhFSUg2M8Ws6ty/e8KLufsxx0VctvwqWvRF6W3Gscp4SY3qdY76js/Kq0Kj+MKPZK1k D69T+Xz9wpitUQ/EB97nDB0N5LXs+Z3XuLjZ/8cv55NqgyPvdu5S+PldRmviX07lFzdQ2l69V 6djab5dcDUxaL1olCD77dfGStcGVCx9NHoWq+Ezdm9VZ+n+yEktxRqKhFnNRcSIAb8BnZaMCA AA= X-Env-Sender: prvs=91881ecf4=Andrew.Cooper3@citrix.com X-Msg-Ref: server-9.tower-21.messagelabs.com!1461776512!11554895!4 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 8.34; banners=-,-,- X-VirusChecked: Checked Received: (qmail 48286 invoked from network); 27 Apr 2016 17:01:57 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-9.tower-21.messagelabs.com with RC4-SHA encrypted SMTP; 27 Apr 2016 17:01:57 -0000 X-IronPort-AV: E=Sophos;i="5.24,542,1454976000"; d="scan'208";a="350200419" From: Andrew Cooper To: Xen-devel Date: Wed, 27 Apr 2016 18:01:22 +0100 Message-ID: <1461776486-31484-4-git-send-email-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1461776486-31484-1-git-send-email-andrew.cooper3@citrix.com> References: <1461776486-31484-1-git-send-email-andrew.cooper3@citrix.com> MIME-Version: 1.0 X-DLP: MIA2 Cc: Andrew Cooper , Ian Jackson , Wei Liu Subject: [Xen-devel] [PATCH for-4.7 3/7] tools/blktap2: Fix array initialisers for tapdisk_disk_{types, drivers}[] X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 Clang points out: tapdisk-disktype.c:117:2: error: initializer overrides prior initialization of this subobject [-Werror,-Winitializer-overrides] 0, ^ tapdisk-disktype.c:115:23: note: previous initialization is here [DISK_TYPE_VINDEX] = &vhd_index_disk, ^~~~~~~~~~~~~~~ Mixing different initialiser styles should be avoided; The actual behaviour is different to the expected behaviour. This specific example has been broken since its introduction in c/s 7b4dea554 "blktap2: Fix tapdisk disktype issues" in 2010, and is caused by the '#if 0' block removing &tapdisk_{sync,vmdk}. First of all, remove what were intended to be trailing NULL entries in tapdisk_disk_{types,drivers}[], making consistent use of Designated Initialisers for the initialisation. This requires changing the loop in tapdisk_disktype_find() to be based on the number of elements in tapdisk_disk_types[], rather than looking for the first NULL. This fixes a latent bug, as the use of Designated Initializers causes to intermediate zero entries if not all indices are explicitly specified. There is a second latent bug where tapdisk_disktype_find() assumes that tapdisk_disk_drivers[] has at least as many entries as tapdisk_disk_types[]. This is not the case and tapdisk_disk_drivers[] had one entry fewer than tapdisk_disk_types[], but the NULL loop bound prevented an out-of-bounds read of tapdisk_disk_drivers[]. Fix the issue by explicitly declaring tapdisk_disk_drivers[] to have the same number of entries as tapdisk_disk_types[]. Finally, this leads to a linker error. It turns out that tapdisk_vhd_index doesn't exist, and I can't find any evidence in the source history to suggest that it ever did. I can only presume that it would have been #if 0'd out like tapdisk_sync and tapdisk_vmdk had it not been for this bug preventing a build failure. Drop all three. No functional change, but only because of the specific layout of tapdisk_disk_types[]. Signed-off-by: Andrew Cooper Acked-by: Wei Liu Reviewed-by: Doug Goldstein --- CC: Ian Jackson CC: Wei Liu Please can someone carefully check my "No functional change" assertion? I am fairly sure it is correct, but this is a gnarly. --- tools/blktap2/drivers/tapdisk-disktype.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/tools/blktap2/drivers/tapdisk-disktype.c b/tools/blktap2/drivers/tapdisk-disktype.c index e9a6890..e89d364 100644 --- a/tools/blktap2/drivers/tapdisk-disktype.c +++ b/tools/blktap2/drivers/tapdisk-disktype.c @@ -33,6 +33,8 @@ #include "tapdisk-disktype.h" #include "tapdisk-message.h" +#define ARRAY_SIZE(a) (sizeof (a) / sizeof *(a)) + static const disk_info_t aio_disk = { "aio", "raw image (aio)", @@ -112,35 +114,25 @@ const disk_info_t *tapdisk_disk_types[] = { [DISK_TYPE_LOG] = &log_disk, [DISK_TYPE_VINDEX] = &vhd_index_disk, [DISK_TYPE_REMUS] = &remus_disk, - 0, }; extern struct tap_disk tapdisk_aio; -extern struct tap_disk tapdisk_sync; -extern struct tap_disk tapdisk_vmdk; extern struct tap_disk tapdisk_vhdsync; extern struct tap_disk tapdisk_vhd; extern struct tap_disk tapdisk_ram; extern struct tap_disk tapdisk_qcow; extern struct tap_disk tapdisk_block_cache; -extern struct tap_disk tapdisk_vhd_index; extern struct tap_disk tapdisk_log; extern struct tap_disk tapdisk_remus; -const struct tap_disk *tapdisk_disk_drivers[] = { +const struct tap_disk *tapdisk_disk_drivers[ARRAY_SIZE(tapdisk_disk_types)] = { [DISK_TYPE_AIO] = &tapdisk_aio, -#if 0 - [DISK_TYPE_SYNC] = &tapdisk_sync, - [DISK_TYPE_VMDK] = &tapdisk_vmdk, -#endif [DISK_TYPE_VHD] = &tapdisk_vhd, [DISK_TYPE_RAM] = &tapdisk_ram, [DISK_TYPE_QCOW] = &tapdisk_qcow, [DISK_TYPE_BLOCK_CACHE] = &tapdisk_block_cache, - [DISK_TYPE_VINDEX] = &tapdisk_vhd_index, [DISK_TYPE_LOG] = &tapdisk_log, [DISK_TYPE_REMUS] = &tapdisk_remus, - 0, }; int @@ -149,7 +141,11 @@ tapdisk_disktype_find(const char *name) const disk_info_t *info; int i; - for (i = 0; info = tapdisk_disk_types[i], info != NULL; ++i) { + for (i = 0; i < ARRAY_SIZE(tapdisk_disk_types); ++i) { + info = tapdisk_disk_types[i]; + if (!info) + continue; + if (strcmp(name, info->name)) continue;