From 67d60824946ade6b1b71a98dbfbfca5949aee425 Mon Sep 17 00:00:00 2001 From: Nikolay Borisov Date: Sat, 10 Sep 2016 23:06:17 +0300 Subject: [PATCH] Refactor spa_load_l2cache to make build happy In case sav->sav_config was NULL the body of the function would skip the iteration of the l2 cache devices and will just cleanup the old devices. However, this wasn't very obvious since the null check was performed after the loop body and after the old devices were cleaned. Refactor the code so that it's now obvious when the iteration of the l2cache devices is skipped. This fixes the following cppcheck warning: [module/zfs/spa.c:1552]: (error) Possible null pointer dereference: newvdevs Reviewed-by: Brian Behlendorf Signed-off-by: Nikolay Borisov Closes #5087 --- module/zfs/spa.c | 57 ++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 463b84d3d..c7cfe6ee8 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -1528,20 +1528,21 @@ spa_load_l2cache(spa_t *spa) ASSERT(spa_config_held(spa, SCL_ALL, RW_WRITER) == SCL_ALL); - if (sav->sav_config != NULL) { - VERIFY(nvlist_lookup_nvlist_array(sav->sav_config, - ZPOOL_CONFIG_L2CACHE, &l2cache, &nl2cache) == 0); - newvdevs = kmem_alloc(nl2cache * sizeof (void *), KM_SLEEP); - } else { - nl2cache = 0; - newvdevs = NULL; - } - oldvdevs = sav->sav_vdevs; oldnvdevs = sav->sav_count; sav->sav_vdevs = NULL; sav->sav_count = 0; + if (sav->sav_config == NULL) { + nl2cache = 0; + newvdevs = NULL; + goto out; + } + + VERIFY(nvlist_lookup_nvlist_array(sav->sav_config, + ZPOOL_CONFIG_L2CACHE, &l2cache, &nl2cache) == 0); + newvdevs = kmem_alloc(nl2cache * sizeof (void *), KM_SLEEP); + /* * Process new nvlist of vdevs. */ @@ -1592,6 +1593,24 @@ spa_load_l2cache(spa_t *spa) } } + sav->sav_vdevs = newvdevs; + sav->sav_count = (int)nl2cache; + + /* + * Recompute the stashed list of l2cache devices, with status + * information this time. + */ + VERIFY(nvlist_remove(sav->sav_config, ZPOOL_CONFIG_L2CACHE, + DATA_TYPE_NVLIST_ARRAY) == 0); + + l2cache = kmem_alloc(sav->sav_count * sizeof (void *), KM_SLEEP); + for (i = 0; i < sav->sav_count; i++) + l2cache[i] = vdev_config_generate(spa, + sav->sav_vdevs[i], B_TRUE, VDEV_CONFIG_L2CACHE); + VERIFY(nvlist_add_nvlist_array(sav->sav_config, + ZPOOL_CONFIG_L2CACHE, l2cache, sav->sav_count) == 0); + +out: /* * Purge vdevs that were dropped */ @@ -1613,26 +1632,6 @@ spa_load_l2cache(spa_t *spa) if (oldvdevs) kmem_free(oldvdevs, oldnvdevs * sizeof (void *)); - if (sav->sav_config == NULL) - goto out; - - sav->sav_vdevs = newvdevs; - sav->sav_count = (int)nl2cache; - - /* - * Recompute the stashed list of l2cache devices, with status - * information this time. - */ - VERIFY(nvlist_remove(sav->sav_config, ZPOOL_CONFIG_L2CACHE, - DATA_TYPE_NVLIST_ARRAY) == 0); - - l2cache = kmem_alloc(sav->sav_count * sizeof (void *), KM_SLEEP); - for (i = 0; i < sav->sav_count; i++) - l2cache[i] = vdev_config_generate(spa, - sav->sav_vdevs[i], B_TRUE, VDEV_CONFIG_L2CACHE); - VERIFY(nvlist_add_nvlist_array(sav->sav_config, - ZPOOL_CONFIG_L2CACHE, l2cache, sav->sav_count) == 0); -out: for (i = 0; i < sav->sav_count; i++) nvlist_free(l2cache[i]); if (sav->sav_count) -- 2.39.5