On Tue, 2011-12-13 at 11:28 -0800, Andy Grover wrote:
> On 12/13/2011 12:26 AM, Dan Carpenter wrote:
> > This isn't your code, but you might know... Smatch complains that
> > we dereference "sess" before checking it.
> >
> > drivers/target/target_core_pr.c +263 target_scsi2_reservation_reserve()
> > warn: variable dereferenced before check 'sess' (see line 248)
> >
> > drivers/target/target_core_pr.c
> > 247 struct se_session *sess = cmd->se_sess;
> > 248 struct se_portal_group *tpg = sess->se_tpg;
> > ^^^^^^^^^^^^
> > Dereference.
>
> <snip>
>
> > 259 /*
> > 260 * This is currently the case for target_core_mod passthrough struct se_cmd
> > 261 * ops
> > 262 */
> > 263 if (!sess || !tpg)
> > ^^^^^
> > Check.
>
> Hi Dan,
>
> I'd say we don't need to check sess.
>
> Furthermore, this is different from the rest of the code when we have to
> do something different for passthrough, we usually check
> dev->transport->transport_type for TRANSPORT_PLUGIN_PHBA_PDEV instead of
> this. I'm not up on this code, so maybe Nick or someone else can give a
> better answer.
>
Hi Dan and Andy,
So these checks where actually not for TRANSPORT_PLUGIN_PHBA_PDEV
backends, but for the legacy v3.x transport_allocate_passthrough() usage
to allowed target-core to submit se_cmd descriptors internally.
As these have been removed some time ago in v4.0 target code, I'll go
ahead and drop these unnecessary checks with the following patch:
Thanks!
--nab
commit 6c163b48b1c6122e2f5e35c0466c710e8aab1c2c
Author: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
Date: Thu Dec 15 23:23:18 2011 -0800
target: Drop legacy passthrough checks in target_core_pr.c
This patch drops a handful of unnecessary se_sess and se_tpg checks in
target_core_pr.c that where originally required for target-core internal
passthrough using transport_allocate_passthrough(), et al that did not
have valid se_cmd->se_sess or se_sess->se_tpg pointers.
This addresses smatch warnings reported by DanC due to pointer dereferencing
before usage.
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 7b34e8c..4d5538c 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -91,11 +91,8 @@ static int core_scsi2_reservation_check(struct se_cmd *cmd, u32 *pr_reg_type)
struct se_session *sess = cmd->se_sess;
int ret;
- if (!sess)
- return 0;
-
spin_lock(&dev->dev_reservation_lock);
- if (!dev->dev_reserved_node_acl || !sess) {
+ if (!dev->dev_reserved_node_acl) {
spin_unlock(&dev->dev_reservation_lock);
return 0;
}
@@ -203,14 +200,12 @@ int target_scsi2_reservation_release(struct se_task *task)
struct se_portal_group *tpg = sess->se_tpg;
int ret = 0;
- if (!sess || !tpg)
- goto out;
if (target_check_scsi2_reservation_conflict(cmd, &ret))
goto out;
ret = 0;
spin_lock(&dev->dev_reservation_lock);
- if (!dev->dev_reserved_node_acl || !sess)
+ if (!dev->dev_reserved_node_acl)
goto out_unlock;
if (dev->dev_reserved_node_acl != sess->se_node_acl)
@@ -253,12 +248,6 @@ int target_scsi2_reservation_reserve(struct se_task *task)
ret = -EINVAL;
goto out;
}
- /*
- * This is currently the case for target_core_mod passthrough struct se_cmd
- * ops
- */
- if (!sess || !tpg)
- goto out;
if (target_check_scsi2_reservation_conflict(cmd, &ret))
goto out;
@@ -581,9 +570,6 @@ static int core_scsi3_pr_reservation_check(
struct se_device *dev = cmd->se_dev;
struct se_session *sess = cmd->se_sess;
int ret;
-
- if (!sess)
- return 0;
/*
* A legacy SPC-2 reservation is being held.
*/
--
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
