Re: [PATCH 2/3] tpm_emulator: drop direct use of errno variable

[Stefan Berger]

On 11/6/25 2:41 PM, Vladimir Sementsov-Ogievskiy wrote:
The code tends to include errno into error messages after
tpm_util_test_tpmdev() and tpm_emulator_ctrlcmd() calls.

Both has error paths, where errno is not set, examples:

Both have ...>
tpm_emulator_ctrlcmd()
   qemu_chr_fe_write_all()
     qemu_chr_write()
       replay_char_write_event_load()
         ...
         *res = replay_get_dword();
         ...

tpm_util_test_tpmdev()
   tpm_util_test()
     tpm_util_request()
       ...
       if (n != requestlen) {
           return -EFAULT;
       }
       ...

Both doesn't document that they set errno.

Both do not ...


Let's drop these explicit usage of errno. If we need this information,
it should be added to errp deeper in the stack.

It's not clear to me why this is an actual problem. Is it better to now 
not set this error message?


Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
  backends/tpm/tpm_emulator.c | 44 ++++++++++++++-----------------------
  1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 24aa18302e..79f3e6b1f2 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -225,8 +225,7 @@ static int tpm_emulator_set_locality(TPMEmulator *tpm_emu, 
uint8_t locty_number,
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_LOCALITY, &loc,
                               sizeof(loc), sizeof(loc.u.resp.tpm_result),
                               sizeof(loc)) < 0) {
-        error_setg(errp, "tpm-emulator: could not set locality : %s",
-                   strerror(errno));
+        error_setg(errp, "tpm-emulator: could not set locality");
          return -1;
      }

@@ -264,7 +263,7 @@ static int tpm_emulator_probe_caps(TPMEmulator *tpm_emu)
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_CAPABILITY, &cap_n, 0,
                               sizeof(cap_n.u.resp.tpm_result),
                               sizeof(cap_n)) < 0) {
-        error_report("tpm-emulator: probing failed : %s", strerror(errno));
+        error_report("tpm-emulator: probing failed");
          return -1;
      }

@@ -315,8 +314,7 @@ static int tpm_emulator_stop_tpm(TPMBackend *tb, Error 
**errp)

      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_STOP, &res, 0,
                               sizeof(ptm_res), sizeof(res)) < 0) {
-        error_setg(errp, "tpm-emulator: Could not stop TPM: %s",
-                   strerror(errno));
+        error_setg(errp, "tpm-emulator: Could not stop TPM");
          return -1;
      }

@@ -344,8 +342,7 @@ static int tpm_emulator_lock_storage(TPMEmulator *tpm_emu)
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_LOCK_STORAGE, &pls, 
sizeof(pls.u.req),
                               sizeof(pls.u.resp.tpm_result),
                               sizeof(pls.u.resp)) < 0) {
-        error_report("tpm-emulator: Could not lock storage within 3 seconds: "
-                     "%s", strerror(errno));
+        error_report("tpm-emulator: Could not lock storage within 3 seconds");
          return -1;
      }

@@ -377,8 +374,7 @@ static int tpm_emulator_set_buffer_size(TPMBackend *tb,
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_BUFFERSIZE, &psbs,
                               sizeof(psbs.u.req), 
sizeof(psbs.u.resp.tpm_result),
                               sizeof(psbs.u.resp)) < 0) {
-        error_setg(errp, "tpm-emulator: Could not set buffer size: %s",
-                   strerror(errno));
+        error_setg(errp, "tpm-emulator: Could not set buffer size");
          return -1;
      }

@@ -426,8 +422,7 @@ static int tpm_emulator_startup_tpm_resume(TPMBackend *tb, 
size_t buffersize,
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_INIT, &init, sizeof(init),
                               sizeof(init.u.resp.tpm_result),
                               sizeof(init)) < 0) {
-        error_setg(errp, "tpm-emulator: could not send INIT: %s",
-                   strerror(errno));
+        error_setg(errp, "tpm-emulator: could not send INIT");
          goto err_exit;
      }

@@ -482,8 +477,7 @@ static bool 
tpm_emulator_get_tpm_established_flag(TPMBackend *tb)
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_GET_TPMESTABLISHED, &est, 0,
                               sizeof(est) /* always returns resp.bit */,
                               sizeof(est)) < 0) {
-        error_report("tpm-emulator: Could not get the TPM established flag: 
%s",
-                     strerror(errno));
+        error_report("tpm-emulator: Could not get the TPM established flag");
          return false;
      }
      trace_tpm_emulator_get_tpm_established_flag(est.u.resp.bit);
@@ -511,8 +505,7 @@ static int 
tpm_emulator_reset_tpm_established_flag(TPMBackend *tb,
                               &reset_est, sizeof(reset_est),
                               sizeof(reset_est.u.resp.tpm_result),
                               sizeof(reset_est)) < 0) {
-        error_report("tpm-emulator: Could not reset the establishment bit: %s",
-                     strerror(errno));
+        error_report("tpm-emulator: Could not reset the establishment bit");
          return -1;
      }

@@ -542,8 +535,7 @@ static void tpm_emulator_cancel_cmd(TPMBackend *tb)
      /* FIXME: make the function non-blocking, or it may block a VCPU */
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_CANCEL_TPM_CMD, &res, 0,
                               sizeof(ptm_res), sizeof(res)) < 0) {
-        error_report("tpm-emulator: Could not cancel command: %s",
-                     strerror(errno));
+        error_report("tpm-emulator: Could not cancel command: %s");
      } else if (res != 0) {
          error_report("tpm-emulator: Failed to cancel TPM: 0x%x",
                       be32_to_cpu(res));
@@ -604,8 +596,7 @@ static int tpm_emulator_prepare_data_fd(TPMEmulator 
*tpm_emu)

      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_DATAFD, &res, 0,
                               sizeof(ptm_res), sizeof(res)) < 0 || res != 0) {
-        error_report("tpm-emulator: Failed to send CMD_SET_DATAFD: %s",
-                     strerror(errno));
+        error_report("tpm-emulator: Failed to send CMD_SET_DATAFD");
          goto err_exit;
      }

@@ -662,8 +653,8 @@ static int tpm_emulator_handle_device_opts(TPMEmulator 
*tpm_emu, QemuOpts *opts)
       */
      if (tpm_util_test_tpmdev(QIO_CHANNEL_SOCKET(tpm_emu->data_ioc)->fd,
                               &tpm_emu->tpm_version)) {
-        error_report("'%s' is not emulating TPM device. Error: %s",
-                      tpm_emu->options->chardev, strerror(errno));
+        error_report("'%s' is not emulating TPM device.",
+                      tpm_emu->options->chardev);
          goto err;
      }

@@ -753,8 +744,7 @@ static int tpm_emulator_get_state_blob(TPMEmulator *tpm_emu,
                               /* always returns up to resp.data */
                               offsetof(ptm_getstate, u.resp.data),
                               offsetof(ptm_getstate, u.resp.data)) < 0) {
-        error_report("tpm-emulator: could not get state blob type %d : %s",
-                     type, strerror(errno));
+        error_report("tpm-emulator: could not get state blob type %d", type);
          return -1;
      }

@@ -856,9 +846,8 @@ static int tpm_emulator_set_state_blob(TPMEmulator *tpm_emu,
      /* write the header only */
      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SET_STATEBLOB, &pss,
                               offsetof(ptm_setstate, u.req.data), 0, 0) < 0) {
-        error_setg_errno(errp, errno,
-                         "tpm-emulator: could not set state blob type %d",
-                         type);
+        error_setg(errp, "tpm-emulator: could not set state blob type %d",
+                   type);
          return -1;
      }

@@ -1040,8 +1029,7 @@ static void tpm_emulator_shutdown(TPMEmulator *tpm_emu)

      if (tpm_emulator_ctrlcmd(tpm_emu, CMD_SHUTDOWN, &res, 0,
                               sizeof(ptm_res), sizeof(res)) < 0) {
-        error_report("tpm-emulator: Could not cleanly shutdown the TPM: %s",
-                     strerror(errno));
+        error_report("tpm-emulator: Could not cleanly shutdown the TPM");
      } else if (res != 0) {
          error_report("tpm-emulator: TPM result for shutdown: 0x%x %s",
                       be32_to_cpu(res), 
tpm_emulator_strerror(be32_to_cpu(res)));