[Qemu-devel] [PATCH v2 0/2] Cleanup and locality range check

[Qemu-devel] [PATCH v2 0/2] Cleanup and locality range check

[Stefan Berger]

This series of patches removes an unnecessary parameter from tpm_tis_abort()
and adds a locality range check to tpm_tis_prep_abort().

   Stefan


Stefan Berger (2):
  tpm: Remove unused locty parameter from tpm_tis_abort()
  tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid

 hw/tpm/tpm_tis.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v2 1/2] tpm: Remove unused locty parameter from tpm_tis_abort()

[Stefan Berger]

Remove the unused locty parameter from tpm_tis_abort() function.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_tis.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d9322692ee..176d424ed9 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -233,7 +233,7 @@ static void tpm_tis_new_active_locality(TPMState *s, uint8_t new_active_locty)
 }
 
 /* abort -- this function switches the locality */
-static void tpm_tis_abort(TPMState *s, uint8_t locty)
+static void tpm_tis_abort(TPMState *s)
 {
     s->rw_offset = 0;
 
@@ -281,7 +281,7 @@ static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, uint8_t newlocty)
         }
     }
 
-    tpm_tis_abort(s, locty);
+    tpm_tis_abort(s);
 }
 
 /*
@@ -311,7 +311,7 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
     }
 
     if (TPM_TIS_IS_VALID_LOCTY(s->next_locty)) {
-        tpm_tis_abort(s, locty);
+        tpm_tis_abort(s);
     }
 
     tpm_tis_raise_irq(s, locty,
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/2] tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid

[Stefan Berger]

Make sure that the new locality passed to tpm_tis_prep_abort()
is valid. This is merely a precaution since none of the current
callers passes an invalid locality number into this function.

Add a comment to aborting_locty that it may be any locality, including
TPM_TIS_NO_LOCALITY.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 hw/tpm/tpm_tis.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 176d424ed9..9b2737b5bf 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -263,7 +263,10 @@ static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, uint8_t newlocty)
 {
     uint8_t busy_locty;
 
-    s->aborting_locty = locty;
+    if (!TPM_TIS_IS_VALID_LOCTY(newlocty))
+        return;
+
+    s->aborting_locty = locty; /* may also be TPM_TIS_NO_LOCALITY */
     s->next_locty = newlocty;  /* locality after successful abort */
 
     /*
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 2/2] tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid

[Marc-André Lureau]

Hi

On Mon, Dec 3, 2018 at 9:01 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
> Make sure that the new locality passed to tpm_tis_prep_abort()
> is valid. This is merely a precaution since none of the current
> callers passes an invalid locality number into this function.

If this should never happen, then let's make it an assert()

> Add a comment to aborting_locty that it may be any locality, including
> TPM_TIS_NO_LOCALITY.
>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>  hw/tpm/tpm_tis.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 176d424ed9..9b2737b5bf 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -263,7 +263,10 @@ static void tpm_tis_prep_abort(TPMState *s, uint8_t locty, uint8_t newlocty)
>  {
>      uint8_t busy_locty;
>
> -    s->aborting_locty = locty;
> +    if (!TPM_TIS_IS_VALID_LOCTY(newlocty))
> +        return;

Follow qemu coding-style, add braces.

> +
> +    s->aborting_locty = locty; /* may also be TPM_TIS_NO_LOCALITY */
>      s->next_locty = newlocty;  /* locality after successful abort */
>
>      /*
> --
> 2.17.1
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 0/2] Cleanup and locality range check

[no-reply]

Patchew URL: https://patchew.org/QEMU/20181203170105.2367492-1-stefanb@linux.vnet.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v2 0/2] Cleanup and locality range check
Message-id: 20181203170105.2367492-1-stefanb@linux.vnet.ibm.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
8e798c4 tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid
b3dfcdf tpm: Remove unused locty parameter from tpm_tis_abort()

=== OUTPUT BEGIN ===
Checking PATCH 1/2: tpm: Remove unused locty parameter from tpm_tis_abort()...
Checking PATCH 2/2: tpm: Make sure new locality passed to tpm_tis_prep_abort() is valid...
ERROR: braces {} are necessary for all arms of this statement
#26: FILE: hw/tpm/tpm_tis.c:266:
+    if (!TPM_TIS_IS_VALID_LOCTY(newlocty))
[...]

total: 1 errors, 0 warnings, 11 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181203170105.2367492-1-stefanb@linux.vnet.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com