[PATCH] dm crypt: remove an impossible condition

[PATCH] dm crypt: remove an impossible condition

[Dan Carpenter]

Static checkers complain that it doesn't make sense to check if "sval"
is NULL.  The intention was to check if strchr() returned NULL, but in
that situation "sval" would be "NULL + 1" so the check doesn't work.  We
know from the sscanf() that there is a ':' character in the string so
the check is unnecessary and can be removed.

Now that the check doesn't depend on "sval" it can be moved earlier
for readability.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index ec09bd703b7d..eba218737bdb 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2401,12 +2401,12 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
 		else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
 			set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
 		else if (sscanf(opt_string, "integrity:%u:", &val) == 1) {
-			cc->on_disk_tag_size = val;
-			sval = strchr(opt_string + strlen("integrity:"), ':') + 1;
-			if (val == 0 || val > MAX_TAG_SIZE || !sval) {
+			if (val == 0 || val > MAX_TAG_SIZE) {
 				ti->error = "Invalid integrity arguments";
 				return -EINVAL;
 			}
+			cc->on_disk_tag_size = val;
+			sval = strchr(opt_string + strlen("integrity:"), ':') + 1;
 			if (!strcasecmp(sval, "aead")) {
 				set_bit(CRYPT_MODE_INTEGRITY_AEAD, &cc->cipher_flags);
 			} else  if (!strncasecmp(sval, "hmac(", strlen("hmac("))) {

[PATCH] dm crypt: remove an impossible condition

[Dan Carpenter]

Static checkers complain that it doesn't make sense to check if "sval"
is NULL.  The intention was to check if strchr() returned NULL, but in
that situation "sval" would be "NULL + 1" so the check doesn't work.  We
know from the sscanf() that there is a ':' character in the string so
the check is unnecessary and can be removed.

Now that the check doesn't depend on "sval" it can be moved earlier
for readability.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index ec09bd703b7d..eba218737bdb 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2401,12 +2401,12 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
 		else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
 			set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
 		else if (sscanf(opt_string, "integrity:%u:", &val) = 1) {
-			cc->on_disk_tag_size = val;
-			sval = strchr(opt_string + strlen("integrity:"), ':') + 1;
-			if (val = 0 || val > MAX_TAG_SIZE || !sval) {
+			if (val = 0 || val > MAX_TAG_SIZE) {
 				ti->error = "Invalid integrity arguments";
 				return -EINVAL;
 			}
+			cc->on_disk_tag_size = val;
+			sval = strchr(opt_string + strlen("integrity:"), ':') + 1;
 			if (!strcasecmp(sval, "aead")) {
 				set_bit(CRYPT_MODE_INTEGRITY_AEAD, &cc->cipher_flags);
 			} else  if (!strncasecmp(sval, "hmac(", strlen("hmac("))) {

Re: dm crypt: remove an impossible condition

[Mike Snitzer]

On Fri, Mar 17 2017 at  4:46pm -0400,
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Static checkers complain that it doesn't make sense to check if "sval"
> is NULL.  The intention was to check if strchr() returned NULL, but in
> that situation "sval" would be "NULL + 1" so the check doesn't work.  We
> know from the sscanf() that there is a ':' character in the string so
> the check is unnecessary and can be removed.
> 
> Now that the check doesn't depend on "sval" it can be moved earlier
> for readability.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Thanks Dan, I've folded this fix into the original commit, see:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.12&id=cf22cd5f3afe7335afee5659f7450000e8fa2a15

I didn't add your Signed-off-by though, I can backfill that if you
like.. it is just I didn't want to taint you with all the extensive
changes in that commit.

Mike

Re: dm crypt: remove an impossible condition

[Mike Snitzer]

On Fri, Mar 17 2017 at  4:46pm -0400,
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Static checkers complain that it doesn't make sense to check if "sval"
> is NULL.  The intention was to check if strchr() returned NULL, but in
> that situation "sval" would be "NULL + 1" so the check doesn't work.  We
> know from the sscanf() that there is a ':' character in the string so
> the check is unnecessary and can be removed.
> 
> Now that the check doesn't depend on "sval" it can be moved earlier
> for readability.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Thanks Dan, I've folded this fix into the original commit, see:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.12&idÏ22cd5f3afe7335afee5659f7450000e8fa2a15

I didn't add your Signed-off-by though, I can backfill that if you
like.. it is just I didn't want to taint you with all the extensive
changes in that commit.

Mike
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: dm crypt: remove an impossible condition

[Dan Carpenter]

On Fri, Mar 17, 2017 at 05:38:19PM -0400, Mike Snitzer wrote:
> On Fri, Mar 17 2017 at  4:46pm -0400,
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > Static checkers complain that it doesn't make sense to check if "sval"
> > is NULL.  The intention was to check if strchr() returned NULL, but in
> > that situation "sval" would be "NULL + 1" so the check doesn't work.  We
> > know from the sscanf() that there is a ':' character in the string so
> > the check is unnecessary and can be removed.
> > 
> > Now that the check doesn't depend on "sval" it can be moved earlier
> > for readability.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Thanks Dan, I've folded this fix into the original commit, see:
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.12&id=cf22cd5f3afe7335afee5659f7450000e8fa2a15
> 
> I didn't add your Signed-off-by though, I can backfill that if you
> like.. it is just I didn't want to taint you with all the extensive
> changes in that commit.

That's fine, thanks.  Sign-off isn't really the appropriate tag anyway...

regards,
dan carpenter

Re: dm crypt: remove an impossible condition

[Dan Carpenter]

On Fri, Mar 17, 2017 at 05:38:19PM -0400, Mike Snitzer wrote:
> On Fri, Mar 17 2017 at  4:46pm -0400,
> Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> > Static checkers complain that it doesn't make sense to check if "sval"
> > is NULL.  The intention was to check if strchr() returned NULL, but in
> > that situation "sval" would be "NULL + 1" so the check doesn't work.  We
> > know from the sscanf() that there is a ':' character in the string so
> > the check is unnecessary and can be removed.
> > 
> > Now that the check doesn't depend on "sval" it can be moved earlier
> > for readability.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Thanks Dan, I've folded this fix into the original commit, see:
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.12&idÏ22cd5f3afe7335afee5659f7450000e8fa2a15
> 
> I didn't add your Signed-off-by though, I can backfill that if you
> like.. it is just I didn't want to taint you with all the extensive
> changes in that commit.

That's fine, thanks.  Sign-off isn't really the appropriate tag anyway...

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html