All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: James Bottomley
	<James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
Cc: linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	open list <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH] tpm: fix a sparse error in tpm-interface.c
Date: Tue, 31 Jan 2017 22:52:09 +0200	[thread overview]
Message-ID: <20170131205209.5552qjqaxjsrycxe@intel.com> (raw)
In-Reply-To: <1485889841.3199.57.camel-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>

On Tue, Jan 31, 2017 at 11:10:41AM -0800, James Bottomley wrote:
> On Tue, 2017-01-31 at 20:07 +0200, Jarkko Sakkinen wrote:
> > drivers/char/tpm//tpm-interface.c:492:42: warning: bad assignment (
> > -=)
> > to restricted __be32
> > 
> > Fixes: 0883743825e3 ("TPM: sysfs functions consolidation")
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > ---
> >  drivers/char/tpm/tpm-interface.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm
> > -interface.c
> > index 423938e..746bc54 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -489,7 +489,7 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32
> > subcap_id, cap_t *cap,
> >  		tpm_cmd.params.getcap_in.cap =
> > cpu_to_be32(subcap_id);
> >  		/*subcap field not necessary */
> >  		tpm_cmd.params.getcap_in.subcap_size =
> > cpu_to_be32(0);
> > -		tpm_cmd.header.in.length -=
> > cpu_to_be32(sizeof(__be32));
> > +		tpm_cmd.header.in.length = cpu_to_be32(18);
> 
> using bare numbers here without comment isn't really best practice. 
>  What about setting header.in.length in both legs of the if() to either
> 
> tpm_cmd.header.in.length = cpu_to_be32(HEADER_SIZE + sizeof(struct tpm_getcap_params_in))
> 
> or 
> 
> /* setting subcap_size to zero allows us to elimnate the subcap field */
> tpm_cmd.header.in.length = cpu_to_be32(HEADER_SIZE + sizeof(struct tpm_getcap_params_in) - sizeof(tpm_cmd.params.getcap_in.subcap))

Yeah, also the commit message should be updated rather than
referring to the static checking tool. I'll update the commit
along the lines you proposed...

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: tpmdd-devel@lists.sourceforge.net,
	open list <linux-kernel@vger.kernel.org>,
	linux-security-module@vger.kernel.org
Subject: Re: [tpmdd-devel] [PATCH] tpm: fix a sparse error in tpm-interface.c
Date: Tue, 31 Jan 2017 22:52:09 +0200	[thread overview]
Message-ID: <20170131205209.5552qjqaxjsrycxe@intel.com> (raw)
In-Reply-To: <1485889841.3199.57.camel@HansenPartnership.com>

On Tue, Jan 31, 2017 at 11:10:41AM -0800, James Bottomley wrote:
> On Tue, 2017-01-31 at 20:07 +0200, Jarkko Sakkinen wrote:
> > drivers/char/tpm//tpm-interface.c:492:42: warning: bad assignment (
> > -=)
> > to restricted __be32
> > 
> > Fixes: 0883743825e3 ("TPM: sysfs functions consolidation")
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  drivers/char/tpm/tpm-interface.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm
> > -interface.c
> > index 423938e..746bc54 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -489,7 +489,7 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32
> > subcap_id, cap_t *cap,
> >  		tpm_cmd.params.getcap_in.cap =
> > cpu_to_be32(subcap_id);
> >  		/*subcap field not necessary */
> >  		tpm_cmd.params.getcap_in.subcap_size =
> > cpu_to_be32(0);
> > -		tpm_cmd.header.in.length -=
> > cpu_to_be32(sizeof(__be32));
> > +		tpm_cmd.header.in.length = cpu_to_be32(18);
> 
> using bare numbers here without comment isn't really best practice. 
>  What about setting header.in.length in both legs of the if() to either
> 
> tpm_cmd.header.in.length = cpu_to_be32(HEADER_SIZE + sizeof(struct tpm_getcap_params_in))
> 
> or 
> 
> /* setting subcap_size to zero allows us to elimnate the subcap field */
> tpm_cmd.header.in.length = cpu_to_be32(HEADER_SIZE + sizeof(struct tpm_getcap_params_in) - sizeof(tpm_cmd.params.getcap_in.subcap))

Yeah, also the commit message should be updated rather than
referring to the static checking tool. I'll update the commit
along the lines you proposed...

/Jarkko

  parent reply	other threads:[~2017-01-31 20:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-31 18:07 [PATCH] tpm: fix a sparse error in tpm-interface.c Jarkko Sakkinen
2017-01-31 18:07 ` Jarkko Sakkinen
     [not found] ` <20170131180742.28460-1-jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-01-31 19:10   ` James Bottomley
2017-01-31 19:10     ` [tpmdd-devel] " James Bottomley
     [not found]     ` <1485889841.3199.57.camel-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
2017-01-31 20:52       ` Jarkko Sakkinen [this message]
2017-01-31 20:52         ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170131205209.5552qjqaxjsrycxe@intel.com \
    --to=jarkko.sakkinen-vuqaysv1563yd54fqh9/ca@public.gmane.org \
    --cc=James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.