On 2025/08/09 23:30, Markus Armbruster wrote:
Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp> writes:

On 2025/08/09 17:17, Markus Armbruster wrote:
Almost missed this, sorry.
Akihiko Odaki <od...@rsg.ci.i.u-tokyo.ac.jp> writes:

On 2025/07/21 22:29, Daniel P. Berrangé wrote:

[...]

No, please do NOT make these functions void. As that text you quote
says, we want functions to return a value indicating success/failure.
'void' return is a historical practice we don't want to continue
in QEMU.

Given that the existing methods all return 'int', we should remain
consistent with the new functions and return 'int', with -1 for
failure, 0 for success, and not use bool.

Markus, I'd also like to hear your opinion since you are the maintainer of the 
error reporting facility.

I'm with Daniel.

New code should stick to the rules.

Changing existing code from "sticks to the rules" to not requires pretty
compelling justification.

The other direction is more welcome, but whether the juice is worth the
squeeze still needs to be decided case by case.

What do you refer with the rules?

The big comment in qapi/error.h starts with a section = Rules =.

There were three options on the table: bool, int, and void.

The previous discussion you referred explains why void should be avoided, and 
include/qapi/error.h also says void should be avoided.

There is pre_load() that does not use Error returns int, but now we are adding 
pre_load_errp() that uses Error.

Then what pre_load_errp() should return: bool or int?

I like bool when it's all we need.

When we need to return a non-negative int on success, use int and return
-1 or a negative error code on failure.

Another reason to pick int is local consistency: related functions
already use int.

Changing working code from int to bool doesn't seem worth the bother.

Questions?


I at least see what "[PATCH v9 26/27] migration: Add error-parameterized function variants in VMSD struct" says is undesirable. It says:
> For the errp variants,
> Returns: 0 on success,
>          <0 on error where -value is an error number from errno.h

There are already non-errp variant implementations that return -1 (e.g., dbus_vmstate_post_load). Adding errp variants that require to return errno is degradation.

I'm still not sure what conclusion will be drawn. I can make two opposite conclusions:

- Non-errp variants return int, so errp variants should also return int
  for local consistency.

- bool is all we need, but changing non-errp variants doesn't seem worth
  the bother, so only errp variants should return bool.

Reply via email to