[PATCH] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD

[PATCH] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD

[Philippe Mathieu-Daudé]

Simplify the tpm_emulator_ctrlcmd() handler by replacing a pair of
qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
macro.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 backends/tpm/tpm_emulator.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index a012adc1934..e5f1063ab6c 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -30,6 +30,7 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qemu/sockets.h"
+#include "qemu/lockable.h"
 #include "io/channel-socket.h"
 #include "sysemu/tpm_backend.h"
 #include "sysemu/tpm_util.h"
@@ -124,31 +125,26 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
     uint32_t cmd_no = cpu_to_be32(cmd);
     ssize_t n = sizeof(uint32_t) + msg_len_in;
     uint8_t *buf = NULL;
-    int ret = -1;
 
-    qemu_mutex_lock(&tpm->mutex);
+    WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
+        buf = g_alloca(n);
+        memcpy(buf, &cmd_no, sizeof(cmd_no));
+        memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
 
-    buf = g_alloca(n);
-    memcpy(buf, &cmd_no, sizeof(cmd_no));
-    memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
-
-    n = qemu_chr_fe_write_all(dev, buf, n);
-    if (n <= 0) {
-        goto end;
-    }
-
-    if (msg_len_out != 0) {
-        n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
+        n = qemu_chr_fe_write_all(dev, buf, n);
         if (n <= 0) {
-            goto end;
+            return -1;
+        }
+
+        if (msg_len_out != 0) {
+            n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
+            if (n <= 0) {
+                return -1;
+            }
         }
     }
 
-    ret = 0;
-
-end:
-    qemu_mutex_unlock(&tpm->mutex);
-    return ret;
+    return 0;
 }
 
 static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
-- 
2.26.3

Re: [PATCH] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD

[Stefan Berger]

On 5/12/21 3:07 AM, Philippe Mathieu-Daudé wrote:
> Simplify the tpm_emulator_ctrlcmd() handler by replacing a pair of
> qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
> macro.
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks for doing this. I suppose you will send it via a PR?

   Stefan

Re: [PATCH] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD

[Philippe Mathieu-Daudé]

Hi Stefan,

On 5/13/21 4:08 PM, Stefan Berger wrote:
> On 5/12/21 3:07 AM, Philippe Mathieu-Daudé wrote:
>> Simplify the tpm_emulator_ctrlcmd() handler by replacing a pair of
>> qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
>> macro.
>>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Thanks for doing this. I suppose you will send it via a PR?

Christophe asked it to be sent separately from the other series.

I Cc'ed qemu-trivial@ (patches are collected there once a week
usually).

Regards,

Phil.

Re: [PATCH] backends/tpm: Replace qemu_mutex_lock calls with QEMU_LOCK_GUARD

[Laurent Vivier]

Le 12/05/2021 à 09:07, Philippe Mathieu-Daudé a écrit :
> Simplify the tpm_emulator_ctrlcmd() handler by replacing a pair of
> qemu_mutex_lock/qemu_mutex_unlock calls by the WITH_QEMU_LOCK_GUARD
> macro.
> 
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  backends/tpm/tpm_emulator.c | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
> index a012adc1934..e5f1063ab6c 100644
> --- a/backends/tpm/tpm_emulator.c
> +++ b/backends/tpm/tpm_emulator.c
> @@ -30,6 +30,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "qemu/sockets.h"
> +#include "qemu/lockable.h"
>  #include "io/channel-socket.h"
>  #include "sysemu/tpm_backend.h"
>  #include "sysemu/tpm_util.h"
> @@ -124,31 +125,26 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm, unsigned long cmd, void *msg,
>      uint32_t cmd_no = cpu_to_be32(cmd);
>      ssize_t n = sizeof(uint32_t) + msg_len_in;
>      uint8_t *buf = NULL;
> -    int ret = -1;
>  
> -    qemu_mutex_lock(&tpm->mutex);
> +    WITH_QEMU_LOCK_GUARD(&tpm->mutex) {
> +        buf = g_alloca(n);
> +        memcpy(buf, &cmd_no, sizeof(cmd_no));
> +        memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
>  
> -    buf = g_alloca(n);
> -    memcpy(buf, &cmd_no, sizeof(cmd_no));
> -    memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
> -
> -    n = qemu_chr_fe_write_all(dev, buf, n);
> -    if (n <= 0) {
> -        goto end;
> -    }
> -
> -    if (msg_len_out != 0) {
> -        n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
> +        n = qemu_chr_fe_write_all(dev, buf, n);
>          if (n <= 0) {
> -            goto end;
> +            return -1;
> +        }
> +
> +        if (msg_len_out != 0) {
> +            n = qemu_chr_fe_read_all(dev, msg, msg_len_out);
> +            if (n <= 0) {
> +                return -1;
> +            }
>          }
>      }
>  
> -    ret = 0;
> -
> -end:
> -    qemu_mutex_unlock(&tpm->mutex);
> -    return ret;
> +    return 0;
>  }
>  
>  static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
> 

Applied to my trivial-patches branch.

Thanks,
Laurent