On 07.11.25 22:31, Stefan Berger wrote:


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?


Using errno directly as source of error for internal QEMU functions is rather 
unusual.
Better pattern is to wrap library functions that uses errno, and return error as
int return code of the function, or even better in errp.
And here, it's at least will have undefined value for some error paths. So, I 
think,
printing it is wrong.

With this commit, we still may lose some correct error information, on failure 
paths
where errno is really set. Does it make sense?

Users should never rely on error messages text in any scenarios, it's just a
"best-effort" information. So, I think code cleanness worth the loss of some 
maybe
useful part of error message.

--
Best regards,
Vladimir

Reply via email to