* [PATCH] dm crypt: remove an impossible condition
@ 2017-03-17 20:46 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-03-17 20:46 UTC (permalink / raw)
To: Alasdair Kergon, Milan Broz
Cc: Mike Snitzer, dm-devel, Shaohua Li, linux-raid, kernel-janitors
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("))) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] dm crypt: remove an impossible condition
@ 2017-03-17 20:46 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-03-17 20:46 UTC (permalink / raw)
To: Alasdair Kergon, Milan Broz
Cc: Mike Snitzer, dm-devel, Shaohua Li, linux-raid, kernel-janitors
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("))) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: dm crypt: remove an impossible condition
2017-03-17 20:46 ` Dan Carpenter
@ 2017-03-17 21:38 ` Mike Snitzer
-1 siblings, 0 replies; 6+ messages in thread
From: Mike Snitzer @ 2017-03-17 21:38 UTC (permalink / raw)
To: Dan Carpenter
Cc: kernel-janitors, linux-raid, dm-devel, Shaohua Li, Milan Broz,
Alasdair Kergon
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: dm crypt: remove an impossible condition
@ 2017-03-17 21:38 ` Mike Snitzer
0 siblings, 0 replies; 6+ messages in thread
From: Mike Snitzer @ 2017-03-17 21:38 UTC (permalink / raw)
To: Dan Carpenter
Cc: kernel-janitors, linux-raid, dm-devel, Shaohua Li, Milan Broz,
Alasdair Kergon
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: dm crypt: remove an impossible condition
2017-03-17 21:38 ` Mike Snitzer
@ 2017-03-18 4:14 ` Dan Carpenter
-1 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-03-18 4:14 UTC (permalink / raw)
To: Mike Snitzer
Cc: kernel-janitors, linux-raid, dm-devel, Shaohua Li, Milan Broz,
Alasdair Kergon
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: dm crypt: remove an impossible condition
@ 2017-03-18 4:14 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2017-03-18 4:14 UTC (permalink / raw)
To: Mike Snitzer
Cc: kernel-janitors, linux-raid, dm-devel, Shaohua Li, Milan Broz,
Alasdair Kergon
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-18 4:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-17 20:46 [PATCH] dm crypt: remove an impossible condition Dan Carpenter
2017-03-17 20:46 ` Dan Carpenter
2017-03-17 21:38 ` Mike Snitzer
2017-03-17 21:38 ` Mike Snitzer
2017-03-18 4:14 ` Dan Carpenter
2017-03-18 4:14 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.