On Wed, 2012-01-11 at 21:43 +0100, Sebastian Andrzej Siewior wrote:
> - core_tpg_pre_addlun()
> returns always ERR_PTR() or the pointer, never NULL. The additional
> check for NULL in core_dev_add_lun() is not required.
>
> - core_tpg_pre_dellun()
> returns always ERR_PTR() or the pointer, never NULL. The check for NULL
> in core_dev_del_lun() is wrong. The third argument (int *) is never
> used, remove it.
>
> - core_dev_add_lun()
> returns always NULL or the pointer, never ERR_PTR. The check for
> IS_ERR() is not required.
>
> In the long term we might want to aim for consistent return codes of
> pointer :)
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> drivers/target/target_core_device.c | 9 ++++-----
> drivers/target/target_core_fabric_configfs.c | 2 +-
> drivers/target/target_core_internal.h | 2 +-
> drivers/target/target_core_tpg.c | 3 +--
> 4 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> index 00159a4..f0baa7c 100644
> --- a/drivers/target/target_core_device.c
> +++ b/drivers/target/target_core_device.c
> @@ -1303,7 +1303,7 @@ struct se_lun *core_dev_add_lun(
> }
>
> lun_p = core_tpg_pre_addlun(tpg, lun);
> - if ((IS_ERR(lun_p)) || !lun_p)
> + if (IS_ERR(lun_p))
> return NULL;
>
> if (dev->dev_flags & DF_READ_ONLY)
> @@ -1349,11 +1349,10 @@ int core_dev_del_lun(
> u32 unpacked_lun)
> {
> struct se_lun *lun;
> - int ret = 0;
>
> - lun = core_tpg_pre_dellun(tpg, unpacked_lun, &ret);
> - if (!lun)
> - return ret;
> + lun = core_tpg_pre_dellun(tpg, unpacked_lun);
> + if (IS_ERR(lun))
> + return PTR_ERR(lun);
>
> core_tpg_post_dellun(tpg, lun);
>
> diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
> index 4f77cce..2066b07 100644
> --- a/drivers/target/target_core_fabric_configfs.c
> +++ b/drivers/target/target_core_fabric_configfs.c
> @@ -766,7 +766,7 @@ static int target_fabric_port_link(
>
> lun_p = core_dev_add_lun(se_tpg, dev->se_hba, dev,
> lun->unpacked_lun);
> - if (IS_ERR(lun_p) || !lun_p) {
> + if (!lun_p) {
> pr_err("core_dev_add_lun() failed\n");
> ret = -EINVAL;
> goto out;
> diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
> index 6ff6708..3a92368 100644
> --- a/drivers/target/target_core_internal.h
> +++ b/drivers/target/target_core_internal.h
> @@ -90,7 +90,7 @@ void core_tpg_wait_for_nacl_pr_ref(struct se_node_acl *);
> struct se_lun *core_tpg_pre_addlun(struct se_portal_group *, u32);
> int core_tpg_post_addlun(struct se_portal_group *, struct se_lun *,
> u32, void *);
> -struct se_lun *core_tpg_pre_dellun(struct se_portal_group *, u32, int *);
> +struct se_lun *core_tpg_pre_dellun(struct se_portal_group *, u32 unpacked_lun);
> int core_tpg_post_dellun(struct se_portal_group *, struct se_lun *);
>
> /* target_core_transport.c */
> diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
> index b766802..06336ec 100644
> --- a/drivers/target/target_core_tpg.c
> +++ b/drivers/target/target_core_tpg.c
> @@ -807,8 +807,7 @@ static void core_tpg_shutdown_lun(
>
> struct se_lun *core_tpg_pre_dellun(
> struct se_portal_group *tpg,
> - u32 unpacked_lun,
> - int *ret)
> + u32 unpacked_lun)
> {
> struct se_lun *lun;
>
Good catches here. Applying this to lio-core/master, and fixing up the
remaining in-consistency mentioned above wrt to core_dev_add_lun()
failure handling.
Thanks Sebastian!
--nab
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
