All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Xen vTPM frontend: Fine-tuning for vtpm_send()
@ 2017-10-22 11:28 ` SF Markus Elfring
  0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-22 11:28 UTC (permalink / raw)
  To: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe, Peter Hüwe
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 22 Oct 2017 13:23:45 +0200

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Use common error handling code
  Combine two condition checks into one statement

 drivers/char/tpm/xen-tpmfront.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

-- 
2.14.2

^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH 0/2] Xen vTPM frontend: Fine-tuning for vtpm_send()
@ 2017-10-22 11:28 ` SF Markus Elfring
  0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-22 11:28 UTC (permalink / raw)
  To: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe, Peter Hüwe
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 22 Oct 2017 13:23:45 +0200

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Use common error handling code
  Combine two condition checks into one statement

 drivers/char/tpm/xen-tpmfront.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

-- 
2.14.2


^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
  2017-10-22 11:28 ` SF Markus Elfring
@ 2017-10-22 11:29   ` SF Markus Elfring
  -1 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-22 11:29 UTC (permalink / raw)
  To: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe, Peter Hüwe
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 22 Oct 2017 12:56:42 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/tpm/xen-tpmfront.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 656e8af95d52..873a5631937a 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -90,10 +90,8 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 
 	/* Wait for completion of any existing command or cancellation */
 	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, chip->timeout_c,
-			&priv->read_queue, true) < 0) {
-		vtpm_cancel(chip);
-		return -ETIME;
-	}
+			      &priv->read_queue, true) < 0)
+		goto cancel_vtpm;
 
 	memcpy(offset + (u8 *)shr, buf, count);
 	shr->length = count;
@@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
 			&priv->read_queue, true) < 0) {
 		/* got a signal or timeout, try to cancel */
-		vtpm_cancel(chip);
-		return -ETIME;
+		goto cancel_vtpm;
 	}
 
 	return count;
+
+cancel_vtpm:
+	vtpm_cancel(chip);
+	return -ETIME;
 }
 
 static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
@ 2017-10-22 11:29   ` SF Markus Elfring
  0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-22 11:29 UTC (permalink / raw)
  To: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe, Peter Hüwe
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 22 Oct 2017 12:56:42 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/tpm/xen-tpmfront.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 656e8af95d52..873a5631937a 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -90,10 +90,8 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 
 	/* Wait for completion of any existing command or cancellation */
 	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, chip->timeout_c,
-			&priv->read_queue, true) < 0) {
-		vtpm_cancel(chip);
-		return -ETIME;
-	}
+			      &priv->read_queue, true) < 0)
+		goto cancel_vtpm;
 
 	memcpy(offset + (u8 *)shr, buf, count);
 	shr->length = count;
@@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
 			&priv->read_queue, true) < 0) {
 		/* got a signal or timeout, try to cancel */
-		vtpm_cancel(chip);
-		return -ETIME;
+		goto cancel_vtpm;
 	}
 
 	return count;
+
+cancel_vtpm:
+	vtpm_cancel(chip);
+	return -ETIME;
 }
 
 static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
-- 
2.14.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
  2017-10-22 11:28 ` SF Markus Elfring
@ 2017-10-22 11:30   ` SF Markus Elfring
  -1 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-22 11:30 UTC (permalink / raw)
  To: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe, Peter Hüwe
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 22 Oct 2017 13:15:16 +0200

A return was performed with the same error code after two condition checks.
Thus use a single statement instead.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/tpm/xen-tpmfront.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 873a5631937a..0d02743e7a87 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -82,10 +82,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 	u32 ordinal;
 	unsigned long duration;
 
-	if (offset > PAGE_SIZE)
-		return -EINVAL;
-
-	if (offset + count > PAGE_SIZE)
+	if (offset > PAGE_SIZE || offset + count > PAGE_SIZE)
 		return -EINVAL;
 
 	/* Wait for completion of any existing command or cancellation */
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
@ 2017-10-22 11:30   ` SF Markus Elfring
  0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-22 11:30 UTC (permalink / raw)
  To: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe, Peter Hüwe
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 22 Oct 2017 13:15:16 +0200

A return was performed with the same error code after two condition checks.
Thus use a single statement instead.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/char/tpm/xen-tpmfront.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 873a5631937a..0d02743e7a87 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -82,10 +82,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
 	u32 ordinal;
 	unsigned long duration;
 
-	if (offset > PAGE_SIZE)
-		return -EINVAL;
-
-	if (offset + count > PAGE_SIZE)
+	if (offset > PAGE_SIZE || offset + count > PAGE_SIZE)
 		return -EINVAL;
 
 	/* Wait for completion of any existing command or cancellation */
-- 
2.14.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
  2017-10-22 11:30   ` SF Markus Elfring
@ 2017-10-23  7:31     ` Dan Carpenter
  -1 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-10-23  7:31 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe,
	Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:30:46PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 13:15:16 +0200
> 
> A return was performed with the same error code after two condition checks.
> Thus use a single statement instead.
> 

It's fine as-is, thanks.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
@ 2017-10-23  7:31     ` Dan Carpenter
  0 siblings, 0 replies; 20+ messages in thread
From: Dan Carpenter @ 2017-10-23  7:31 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jarkko Sakkinen, Jason Gunthorpe,
	Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:30:46PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 13:15:16 +0200
> 
> A return was performed with the same error code after two condition checks.
> Thus use a single statement instead.
> 

It's fine as-is, thanks.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 0/2] Xen vTPM frontend: Fine-tuning for vtpm_send()
  2017-10-22 11:28 ` SF Markus Elfring
@ 2017-10-23 13:42   ` Jarkko Sakkinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-23 13:42 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:28:14PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 13:23:45 +0200
> 
> Two update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (2):
>   Use common error handling code
>   Combine two condition checks into one statement
> 
>  drivers/char/tpm/xen-tpmfront.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> -- 
> 2.14.2
> 

Useless cover letter. Two separate unrelated patches.

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 0/2] Xen vTPM frontend: Fine-tuning for vtpm_send()
@ 2017-10-23 13:42   ` Jarkko Sakkinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-23 13:42 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:28:14PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 13:23:45 +0200
> 
> Two update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (2):
>   Use common error handling code
>   Combine two condition checks into one statement
> 
>  drivers/char/tpm/xen-tpmfront.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> -- 
> 2.14.2
> 

Useless cover letter. Two separate unrelated patches.

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
  2017-10-22 11:29   ` SF Markus Elfring
@ 2017-10-23 13:42     ` Jarkko Sakkinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-23 13:42 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:29:33PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 12:56:42 +0200
> 
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/char/tpm/xen-tpmfront.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 656e8af95d52..873a5631937a 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -90,10 +90,8 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>  
>  	/* Wait for completion of any existing command or cancellation */
>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, chip->timeout_c,
> -			&priv->read_queue, true) < 0) {
> -		vtpm_cancel(chip);
> -		return -ETIME;
> -	}
> +			      &priv->read_queue, true) < 0)
> +		goto cancel_vtpm;
>  
>  	memcpy(offset + (u8 *)shr, buf, count);
>  	shr->length = count;
> @@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
>  			&priv->read_queue, true) < 0) {
>  		/* got a signal or timeout, try to cancel */
> -		vtpm_cancel(chip);
> -		return -ETIME;
> +		goto cancel_vtpm;
>  	}
>  
>  	return count;
> +
> +cancel_vtpm:
> +	vtpm_cancel(chip);
> +	return -ETIME;
>  }
>  
>  static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> -- 
> 2.14.2
> 

NAK

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
@ 2017-10-23 13:42     ` Jarkko Sakkinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-23 13:42 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:29:33PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 12:56:42 +0200
> 
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/char/tpm/xen-tpmfront.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 656e8af95d52..873a5631937a 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -90,10 +90,8 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>  
>  	/* Wait for completion of any existing command or cancellation */
>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, chip->timeout_c,
> -			&priv->read_queue, true) < 0) {
> -		vtpm_cancel(chip);
> -		return -ETIME;
> -	}
> +			      &priv->read_queue, true) < 0)
> +		goto cancel_vtpm;
>  
>  	memcpy(offset + (u8 *)shr, buf, count);
>  	shr->length = count;
> @@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
>  			&priv->read_queue, true) < 0) {
>  		/* got a signal or timeout, try to cancel */
> -		vtpm_cancel(chip);
> -		return -ETIME;
> +		goto cancel_vtpm;
>  	}
>  
>  	return count;
> +
> +cancel_vtpm:
> +	vtpm_cancel(chip);
> +	return -ETIME;
>  }
>  
>  static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> -- 
> 2.14.2
> 

NAK

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
  2017-10-22 11:30   ` SF Markus Elfring
@ 2017-10-23 13:44     ` Jarkko Sakkinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-23 13:44 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:30:46PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 13:15:16 +0200
> 
> A return was performed with the same error code after two condition checks.
> Thus use a single statement instead.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/char/tpm/xen-tpmfront.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 873a5631937a..0d02743e7a87 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -82,10 +82,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>  	u32 ordinal;
>  	unsigned long duration;
>  
> -	if (offset > PAGE_SIZE)
> -		return -EINVAL;
> -
> -	if (offset + count > PAGE_SIZE)
> +	if (offset > PAGE_SIZE || offset + count > PAGE_SIZE)
>  		return -EINVAL;
>  
>  	/* Wait for completion of any existing command or cancellation */
> -- 
> 2.14.2
> 

This is fine.

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
@ 2017-10-23 13:44     ` Jarkko Sakkinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-23 13:44 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Sun, Oct 22, 2017 at 01:30:46PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 22 Oct 2017 13:15:16 +0200
> 
> A return was performed with the same error code after two condition checks.
> Thus use a single statement instead.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/char/tpm/xen-tpmfront.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> index 873a5631937a..0d02743e7a87 100644
> --- a/drivers/char/tpm/xen-tpmfront.c
> +++ b/drivers/char/tpm/xen-tpmfront.c
> @@ -82,10 +82,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>  	u32 ordinal;
>  	unsigned long duration;
>  
> -	if (offset > PAGE_SIZE)
> -		return -EINVAL;
> -
> -	if (offset + count > PAGE_SIZE)
> +	if (offset > PAGE_SIZE || offset + count > PAGE_SIZE)
>  		return -EINVAL;
>  
>  	/* Wait for completion of any existing command or cancellation */
> -- 
> 2.14.2
> 

This is fine.

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
  2017-10-23 13:42     ` Jarkko Sakkinen
@ 2017-10-23 13:57       ` SF Markus Elfring
  -1 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-23 13:57 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

>> @@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
>>  			&priv->read_queue, true) < 0) {
>>  		/* got a signal or timeout, try to cancel */
>> -		vtpm_cancel(chip);
>> -		return -ETIME;
>> +		goto cancel_vtpm;
>>  	}
>>  
>>  	return count;
>> +
>> +cancel_vtpm:
>> +	vtpm_cancel(chip);
>> +	return -ETIME;
>>  }
>>  
>>  static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>> -- 
>> 2.14.2
>>
> 
> NAK

Do you need any more facts to show the influence of the proposed small code reduction?

Regards,
Markus

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
@ 2017-10-23 13:57       ` SF Markus Elfring
  0 siblings, 0 replies; 20+ messages in thread
From: SF Markus Elfring @ 2017-10-23 13:57 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

>> @@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
>>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
>>  			&priv->read_queue, true) < 0) {
>>  		/* got a signal or timeout, try to cancel */
>> -		vtpm_cancel(chip);
>> -		return -ETIME;
>> +		goto cancel_vtpm;
>>  	}
>>  
>>  	return count;
>> +
>> +cancel_vtpm:
>> +	vtpm_cancel(chip);
>> +	return -ETIME;
>>  }
>>  
>>  static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
>> -- 
>> 2.14.2
>>
> 
> NAK

Do you need any more facts to show the influence of the proposed small code reduction?

Regards,
Markus

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
  2017-10-23 13:44     ` Jarkko Sakkinen
@ 2017-10-24 13:52       ` Jarkko Sakkinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-24 13:52 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Mon, Oct 23, 2017 at 03:44:01PM +0200, Jarkko Sakkinen wrote:
> On Sun, Oct 22, 2017 at 01:30:46PM +0200, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 22 Oct 2017 13:15:16 +0200
> > 
> > A return was performed with the same error code after two condition checks.
> > Thus use a single statement instead.
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/char/tpm/xen-tpmfront.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> > index 873a5631937a..0d02743e7a87 100644
> > --- a/drivers/char/tpm/xen-tpmfront.c
> > +++ b/drivers/char/tpm/xen-tpmfront.c
> > @@ -82,10 +82,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> >  	u32 ordinal;
> >  	unsigned long duration;
> >  
> > -	if (offset > PAGE_SIZE)
> > -		return -EINVAL;
> > -
> > -	if (offset + count > PAGE_SIZE)
> > +	if (offset > PAGE_SIZE || offset + count > PAGE_SIZE)
> >  		return -EINVAL;
> >  
> >  	/* Wait for completion of any existing command or cancellation */
> > -- 
> > 2.14.2
> > 
> 
> This is fine.
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> /Jarkko

Anyway I NAK it because it is really a change of insignificant value.
Just adds cruft to the kernel release.

I would be fine if someone did this as part of something significant.

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement in vtpm_send()
@ 2017-10-24 13:52       ` Jarkko Sakkinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-24 13:52 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Mon, Oct 23, 2017 at 03:44:01PM +0200, Jarkko Sakkinen wrote:
> On Sun, Oct 22, 2017 at 01:30:46PM +0200, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 22 Oct 2017 13:15:16 +0200
> > 
> > A return was performed with the same error code after two condition checks.
> > Thus use a single statement instead.
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >  drivers/char/tpm/xen-tpmfront.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
> > index 873a5631937a..0d02743e7a87 100644
> > --- a/drivers/char/tpm/xen-tpmfront.c
> > +++ b/drivers/char/tpm/xen-tpmfront.c
> > @@ -82,10 +82,7 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> >  	u32 ordinal;
> >  	unsigned long duration;
> >  
> > -	if (offset > PAGE_SIZE)
> > -		return -EINVAL;
> > -
> > -	if (offset + count > PAGE_SIZE)
> > +	if (offset > PAGE_SIZE || offset + count > PAGE_SIZE)
> >  		return -EINVAL;
> >  
> >  	/* Wait for completion of any existing command or cancellation */
> > -- 
> > 2.14.2
> > 
> 
> This is fine.
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> /Jarkko

Anyway I NAK it because it is really a change of insignificant value.
Just adds cruft to the kernel release.

I would be fine if someone did this as part of something significant.

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
  2017-10-23 13:57       ` SF Markus Elfring
@ 2017-10-24 13:56         ` Jarkko Sakkinen
  -1 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-24 13:56 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Mon, Oct 23, 2017 at 03:57:30PM +0200, SF Markus Elfring wrote:
> >> @@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> >>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
> >>  			&priv->read_queue, true) < 0) {
> >>  		/* got a signal or timeout, try to cancel */
> >> -		vtpm_cancel(chip);
> >> -		return -ETIME;
> >> +		goto cancel_vtpm;
> >>  	}
> >>  
> >>  	return count;
> >> +
> >> +cancel_vtpm:
> >> +	vtpm_cancel(chip);
> >> +	return -ETIME;
> >>  }
> >>  
> >>  static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> >> -- 
> >> 2.14.2
> >>
> > 
> > NAK
> 
> Do you need any more facts to show the influence of the proposed small code reduction?
> 
> Regards,
> Markus

Already explained when I reviewed another patch dealing with
-ETIME.

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send()
@ 2017-10-24 13:56         ` Jarkko Sakkinen
  0 siblings, 0 replies; 20+ messages in thread
From: Jarkko Sakkinen @ 2017-10-24 13:56 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-integrity, Jason Gunthorpe, Peter Hüwe, LKML, kernel-janitors

On Mon, Oct 23, 2017 at 03:57:30PM +0200, SF Markus Elfring wrote:
> >> @@ -108,11 +106,14 @@ static int vtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> >>  	if (wait_for_tpm_stat(chip, VTPM_STATUS_IDLE, duration,
> >>  			&priv->read_queue, true) < 0) {
> >>  		/* got a signal or timeout, try to cancel */
> >> -		vtpm_cancel(chip);
> >> -		return -ETIME;
> >> +		goto cancel_vtpm;
> >>  	}
> >>  
> >>  	return count;
> >> +
> >> +cancel_vtpm:
> >> +	vtpm_cancel(chip);
> >> +	return -ETIME;
> >>  }
> >>  
> >>  static int vtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> >> -- 
> >> 2.14.2
> >>
> > 
> > NAK
> 
> Do you need any more facts to show the influence of the proposed small code reduction?
> 
> Regards,
> Markus

Already explained when I reviewed another patch dealing with
-ETIME.

/Jarkko

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-10-24 13:56 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-22 11:28 [PATCH 0/2] Xen vTPM frontend: Fine-tuning for vtpm_send() SF Markus Elfring
2017-10-22 11:28 ` SF Markus Elfring
2017-10-22 11:29 ` [PATCH 1/2] xen-tpmfront: Use common error handling code in vtpm_send() SF Markus Elfring
2017-10-22 11:29   ` SF Markus Elfring
2017-10-23 13:42   ` Jarkko Sakkinen
2017-10-23 13:42     ` Jarkko Sakkinen
2017-10-23 13:57     ` SF Markus Elfring
2017-10-23 13:57       ` SF Markus Elfring
2017-10-24 13:56       ` Jarkko Sakkinen
2017-10-24 13:56         ` Jarkko Sakkinen
2017-10-22 11:30 ` [PATCH 2/2] xen-tpmfront: Combine two condition checks into one statement " SF Markus Elfring
2017-10-22 11:30   ` SF Markus Elfring
2017-10-23  7:31   ` Dan Carpenter
2017-10-23  7:31     ` Dan Carpenter
2017-10-23 13:44   ` Jarkko Sakkinen
2017-10-23 13:44     ` Jarkko Sakkinen
2017-10-24 13:52     ` Jarkko Sakkinen
2017-10-24 13:52       ` Jarkko Sakkinen
2017-10-23 13:42 ` [PATCH 0/2] Xen vTPM frontend: Fine-tuning for vtpm_send() Jarkko Sakkinen
2017-10-23 13:42   ` Jarkko Sakkinen

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.