Philippe Mathieu-Daudé <phi...@redhat.com> writes:
> Commits 1c94a35164..7b3cb8037c simplified the error propagation.
The complete series is b6d7e9b66f..a43770df5d. The part you quoted
omits half of the transformation for qemu-option and QAPI. The other
half is in
a5f9b9df25 error: Reduce unnecessary error propagation
992861fb1e error: Eliminate error_propagate() manually
af175e85f9 error: Eliminate error_propagate() with Coccinelle, part 2
668f62ec62 error: Eliminate error_propagate() with Coccinelle, part 1
dcfe480544 error: Avoid unnecessary error_propagate() after error_setg()
I'd simply point to the complete series.
> Similarly to commit 73ac1aac39 ("qdev: Make functions taking
> Error ** return bool, not void")
I think commit 6fd5bef10b "qom: Make functions taking Error ** return
bool, not void" would be a closer match.
> let the ObjectPropertyGet
> functions return a boolean value, not void.
The conversion simplifies most calls at the cost of slightly
complicating the callee. The complete series quoted above shows it can
be a good trade:
276 files changed, 2424 insertions(+), 3567 deletions(-)
> See commit e3fe3988d7 ("error: Document Error API usage rules")
> for rationale.
>
> Cc: arm...@redhat.com
> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> ---
> Sorry I don't see how to split that patch without using
> ugly casts in the middle.
> ---
[...]
> 57 files changed, 325 insertions(+), 281 deletions(-)
This one's different: it converts a callback. Many more functions than
calls.
It still improves consistency.
It should also help reduce Coverity CHECKED_RETURN false positives in
getters; see below.
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index e9496ba970..7ba2172932 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -333,9 +333,11 @@ typedef void (ObjectPropertySet)(Object *obj,
> * @opaque: the object property opaque
> * @errp: a pointer to an Error that is filled if getting fails.
> *
> + * Return true on success, false on failure.
> + *
> * Called when trying to get a property.
> */
> -typedef void (ObjectPropertyGet)(Object *obj,
> +typedef bool (ObjectPropertyGet)(Object *obj,
> Visitor *v,
> const char *name,
> void *opaque,
[...]
> diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> index 08aede88dc..d59eadd4da 100644
> --- a/target/ppc/compat.c
> +++ b/target/ppc/compat.c
> @@ -238,7 +238,7 @@ int ppc_compat_max_vthreads(PowerPCCPU *cpu)
> return n_threads;
> }
>
> -static void ppc_compat_prop_get(Object *obj, Visitor *v, const char *name,
> +static bool ppc_compat_prop_get(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> uint32_t compat_pvr = *((uint32_t *)opaque);
> @@ -254,7 +254,7 @@ static void ppc_compat_prop_get(Object *obj, Visitor *v,
> const char *name,
> value = compat->name;
> }
>
> - visit_type_str(v, name, (char **)&value, errp);
> + return visit_type_str(v, name, (char **)&value, errp);
Before the patch, Coverity reports
CID 1430435: Error handling issues (CHECKED_RETURN)
Calling "visit_type_str" without checking return value (as is done elsewhere
531 out of 561 times).
False positive; not checking is fine here. Still, avoiding false
positives is nice, as long as it can be done without impairing
readability.
> }
>
> static void ppc_compat_prop_set(Object *obj, Visitor *v, const char *name,
[...]
