[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen master] tools/blktap2: Fix array initialisers for tapdisk_disk_{types, drivers}[]



commit 05fc4ef8ae3e99e8a26cffb2bc9ea22c306fb5fe
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Wed Apr 27 11:23:04 2016 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Wed Apr 27 20:31:08 2016 +0100

    tools/blktap2: Fix array initialisers for tapdisk_disk_{types,drivers}[]
    
    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 <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
    Release-acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
    Reviewed-by: Doug Goldstein <cardoe@xxxxxxxxxx>
---
 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;
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.