Stefan Berger <[email protected]> writes:

> 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?

Error messages lacking information are bad.  Error messages with
incorrect information are *worse*.  When the error message lacks
information I need, I usually realize it immediately.  When it lies to
me, I don't.

Before this patch, correct information is mixed up with incorrect
information.  At the point where we format it into error messages, we
have no idea whether it's correct.  Thus, the error messages lie at
least some of the time.  That's a bug.

The patch fixes that bug at the cost of losing some correct information
along with the lies.

If you'd rather lose just the lies, that's possible, but more involved.
As the saying goes: patches welcome!

[...]


Reply via email to