aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2020-07-10virtio-9p: Use ERRP_GUARD()Vladimir Sementsov-Ogievskiy2-7/+6
If we want to check error after errp-function call, we need to introduce local_err and then propagate it to errp. Instead, use the ERRP_GUARD() macro, benefits are: 1. No need of explicit error_propagate call 2. No need of explicit local_err variable: use errp directly 3. ERRP_GUARD() leaves errp as is if it's not NULL or &error_fatal, this means that we don't break error_abort (we'll abort on error_set, not on error_propagate) If we want to add some info to errp (by error_prepend() or error_append_hint()), we must use the ERRP_GUARD() macro. Otherwise, this info will not be added when errp == &error_fatal (the program will exit prior to the error_append_hint() or error_prepend() call). Fix such a case in v9fs_device_realize_common(). This commit is generated by command sed -n '/^virtio-9p$/,/^$/{s/^F: //p}' MAINTAINERS | \ xargs git ls-files | grep '\.[hc]$' | \ xargs spatch \ --sp-file scripts/coccinelle/errp-guard.cocci \ --macro-file scripts/cocci-macro-file.h \ --in-place --no-show-diff --max-width 80 Reported-by: Kevin Wolf <kwolf@redhat.com> Reported-by: Greg Kurz <groug@kaod.org> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Acked-by: Greg Kurz <groug@kaod.org> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707165037.1026246-7-armbru@redhat.com> [ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and auto-propagated-errp.cocci to errp-guard.cocci. Commit message tweaked again.]
2020-07-10fw_cfg: Use ERRP_GUARD()Vladimir Sementsov-Ogievskiy1-12/+9
If we want to check error after errp-function call, we need to introduce local_err and then propagate it to errp. Instead, use the ERRP_GUARD() macro, benefits are: 1. No need of explicit error_propagate call 2. No need of explicit local_err variable: use errp directly 3. ERRP_GUARD() leaves errp as is if it's not NULL or &error_fatal, this means that we don't break error_abort (we'll abort on error_set, not on error_propagate) If we want to add some info to errp (by error_prepend() or error_append_hint()), we must use the ERRP_GUARD() macro. Otherwise, this info will not be added when errp == &error_fatal (the program will exit prior to the error_append_hint() or error_prepend() call). No such cases are being fixed here. This commit is generated by command sed -n '/^Firmware configuration (fw_cfg)$/,/^$/{s/^F: //p}' \ MAINTAINERS | \ xargs git ls-files | grep '\.[hc]$' | \ xargs spatch \ --sp-file scripts/coccinelle/errp-guard.cocci \ --macro-file scripts/cocci-macro-file.h \ --in-place --no-show-diff --max-width 80 Reported-by: Kevin Wolf <kwolf@redhat.com> Reported-by: Greg Kurz <groug@kaod.org> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707165037.1026246-6-armbru@redhat.com> [ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and auto-propagated-errp.cocci to errp-guard.cocci. Commit message tweaked again. Coccinelle script rerun for commit 3203148917 "hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface"]
2020-07-10pflash: Use ERRP_GUARD()Vladimir Sementsov-Ogievskiy2-8/+6
If we want to check error after errp-function call, we need to introduce local_err and then propagate it to errp. Instead, use the ERRP_GUARD() macro, benefits are: 1. No need of explicit error_propagate call 2. No need of explicit local_err variable: use errp directly 3. ERRP_GUARD() leaves errp as is if it's not NULL or &error_fatal, this means that we don't break error_abort (we'll abort on error_set, not on error_propagate) If we want to add some info to errp (by error_prepend() or error_append_hint()), we must use the ERRP_GUARD() macro. Otherwise, this info will not be added when errp == &error_fatal (the program will exit prior to the error_append_hint() or error_prepend() call). No such cases are being fixed here. This commit is generated by command sed -n '/^Parallel NOR Flash devices$/,/^$/{s/^F: //p}' \ MAINTAINERS | \ xargs git ls-files | grep '\.[hc]$' | \ xargs spatch \ --sp-file scripts/coccinelle/errp-guard.cocci \ --macro-file scripts/cocci-macro-file.h \ --in-place --no-show-diff --max-width 80 Reported-by: Kevin Wolf <kwolf@redhat.com> Reported-by: Greg Kurz <groug@kaod.org> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707165037.1026246-5-armbru@redhat.com> [ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and auto-propagated-errp.cocci to errp-guard.cocci. Commit message tweaked again.]
2020-07-10sd: Use ERRP_GUARD()Vladimir Sementsov-Ogievskiy3-21/+17
If we want to check error after errp-function call, we need to introduce local_err and then propagate it to errp. Instead, use the ERRP_GUARD() macro, benefits are: 1. No need of explicit error_propagate call 2. No need of explicit local_err variable: use errp directly 3. ERRP_GUARD() leaves errp as is if it's not NULL or &error_fatal, this means that we don't break error_abort (we'll abort on error_set, not on error_propagate) If we want to add some info to errp (by error_prepend() or error_append_hint()), we must use the ERRP_GUARD() macro. Otherwise, this info will not be added when errp == &error_fatal (the program will exit prior to the error_append_hint() or error_prepend() call). No such cases are being fixed here. This commit is generated by command sed -n '/^SD (Secure Card)$/,/^$/{s/^F: //p}' \ MAINTAINERS | \ xargs git ls-files | grep '\.[hc]$' | \ xargs spatch \ --sp-file scripts/coccinelle/errp-guard.cocci \ --macro-file scripts/cocci-macro-file.h \ --in-place --no-show-diff --max-width 80 Reported-by: Kevin Wolf <kwolf@redhat.com> Reported-by: Greg Kurz <groug@kaod.org> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707165037.1026246-4-armbru@redhat.com> [ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and auto-propagated-errp.cocci to errp-guard.cocci. Commit message tweaked again.]
2020-07-10scripts: Coccinelle script to use ERRP_GUARD()Vladimir Sementsov-Ogievskiy3-0/+339
Script adds ERRP_GUARD() macro invocations where appropriate and does corresponding changes in code (look for details in include/qapi/error.h) Usage example: spatch --sp-file scripts/coccinelle/errp-guard.cocci \ --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \ --max-width 80 FILES... Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707165037.1026246-3-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> [ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and auto-propagated-errp.cocci to errp-guard.cocci]
2020-07-10error: New macro ERRP_GUARD()Vladimir Sementsov-Ogievskiy1-19/+139
Introduce a new ERRP_GUARD() macro, to be used at start of functions with an errp OUT parameter. It has three goals: 1. Fix issue with error_fatal and error_prepend/error_append_hint: the user can't see this additional information, because exit() happens in error_setg earlier than information is added. [Reported by Greg Kurz] 2. Fix issue with error_abort and error_propagate: when we wrap error_abort by local_err+error_propagate, the resulting coredump will refer to error_propagate and not to the place where error happened. (the macro itself doesn't fix the issue, but it allows us to [3.] drop the local_err+error_propagate pattern, which will definitely fix the issue) [Reported by Kevin Wolf] 3. Drop local_err+error_propagate pattern, which is used to workaround void functions with errp parameter, when caller wants to know resulting status. (Note: actually these functions could be merely updated to return int error code). To achieve these goals, later patches will add invocations of this macro at the start of functions with either use error_prepend/error_append_hint (solving 1) or which use local_err+error_propagate to check errors, switching those functions to use *errp instead (solving 2 and 3). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Eric Blake <eblake@redhat.com> [Merge comments properly with recent commit "error: Document Error API usage rules", and edit for clarity. Put ERRP_AUTO_PROPAGATE() before its helpers, and touch up style. Tweak commit message.] Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707165037.1026246-2-armbru@redhat.com> [Rename ERRP_AUTO_PROPAGATE() to ERRP_GUARD(), tweak commit message again]
2020-07-10hmp: Ignore Error objects where the return value sufficesMarkus Armbruster1-6/+6
qdev_print_props() receives and throws away Error objects just to check for object_property_get_str() and object_property_print() failure. Unnecessary, both return suitable values, so use those instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-46-armbru@redhat.com>
2020-07-10qdev: Ignore Error objects where the return value sufficesMarkus Armbruster1-4/+1
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-45-armbru@redhat.com>
2020-07-10qemu-img: Ignore Error objects where the return value sufficesMarkus Armbruster1-12/+3
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-44-armbru@redhat.com> [One more in img_amend() due to commit 0bc2a50e17 "qemu-option: Use returned bool to check for failure"]
2020-07-10error: Avoid error_propagate() after migrate_add_blocker()Markus Armbruster13-45/+20
When migrate_add_blocker(blocker, &errp) is followed by error_propagate(errp, err), we can often just as well do migrate_add_blocker(..., errp). Do that with this Coccinelle script: @@ expression blocker, err, errp; expression ret; @@ - ret = migrate_add_blocker(blocker, &err); - if (err) { + ret = migrate_add_blocker(blocker, errp); + if (ret < 0) { ... when != err; - error_propagate(errp, err); ... } @@ expression blocker, err, errp; @@ - migrate_add_blocker(blocker, &err); - if (err) { + if (migrate_add_blocker(blocker, errp) < 0) { ... when != err; - error_propagate(errp, err); ... } Double-check @err is not used afterwards. Dereferencing it would be use after free, but checking whether it's null would be legitimate. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-43-armbru@redhat.com>
2020-07-10qapi: Purge error_propagate() from QAPI coreMarkus Armbruster1-21/+19
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-42-armbru@redhat.com>
2020-07-10qapi: Smooth visitor error checking in generated codeMarkus Armbruster3-84/+55
Use visitor functions' return values to check for failure. Eliminate error_propagate() that are now unnecessary. Delete @err that are now unused. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-41-armbru@redhat.com>
2020-07-10qapi: Smooth another visitor error checking patternMarkus Armbruster15-78/+35
Convert visit_type_FOO(v, ..., &ptr, &err); ... if (err) { ... } to visit_type_FOO(v, ..., &ptr, errp); ... if (!ptr) { ... } for functions that set @ptr to non-null / null on success / error. Eliminate error_propagate() that are now unnecessary. Delete @err that are now unused. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-40-armbru@redhat.com>
2020-07-10block/parallels: Simplify parallels_open() after previous commitMarkus Armbruster1-5/+2
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-39-armbru@redhat.com>
2020-07-10error: Reduce unnecessary error propagationMarkus Armbruster8-14/+13
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away, even when we need to keep error_propagate() for other error paths. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-38-armbru@redhat.com>
2020-07-10error: Eliminate error_propagate() manuallyMarkus Armbruster18-149/+67
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous two commits did that for sufficiently simple cases with Coccinelle. Do it for several more manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-37-armbru@redhat.com>
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 2Markus Armbruster23-77/+32
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. The previous commit did that with a Coccinelle script I consider fairly trustworthy. This commit uses the same script with the matching of return taken out, i.e. we convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... } to if (!foo(..., errp)) { ... ... } This is unsound: @err could still be read between afterwards. I don't know how to express "no read of @err without an intervening write" in Coccinelle. Instead, I manually double-checked for uses of @err. Suboptimal line breaks tweaked manually. qdev_realize() simplified further to placate scripts/checkpatch.pl. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-36-armbru@redhat.com>
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 1Markus Armbruster114-896/+383
When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-35-armbru@redhat.com>
2020-07-10error: Avoid unnecessary error_propagate() after error_setg()Markus Armbruster24-242/+169
Replace error_setg(&err, ...); error_propagate(errp, err); by error_setg(errp, ...); Related pattern: if (...) { error_setg(&err, ...); goto out; } ... out: error_propagate(errp, err); return; When all paths to label out are that way, replace by if (...) { error_setg(errp, ...); return; } and delete the label along with the error_propagate(). When we have at most one other path that actually needs to propagate, and maybe one at the end that where propagation is unnecessary, e.g. foo(..., &err); if (err) { goto out; } ... bar(..., &err); out: error_propagate(errp, err); return; move the error_propagate() to where it's needed, like if (...) { foo(..., &err); error_propagate(errp, err); return; } ... bar(..., errp); return; and transform the error_setg() as above. In some places, the transformation results in obviously unnecessary error_propagate(). The next few commits will eliminate them. Bonus: the elimination of gotos will make later patches in this series easier to review. Candidates for conversion tracked down with this Coccinelle script: @@ identifier err, errp; expression list args; @@ - error_setg(&err, args); + error_setg(errp, args); ... when != err error_propagate(errp, err); Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-34-armbru@redhat.com>
2020-07-10qdev: Use returned bool to check for failure, Coccinelle partMarkus Armbruster3-7/+4
The previous commit enables conversion of qdev_prop_set_drive_err(..., &err); if (err) { ... } to if (!qdev_prop_set_drive_err(..., errp)) { ... } Coccinelle script: @@ identifier fun = qdev_prop_set_drive_err; expression list args; typedef Error; Error *err; @@ - fun(args, &err); - if (err) + if (!fun(args, &err)) { ... } One line break tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-33-armbru@redhat.com>
2020-07-10qdev: Make functions taking Error ** return bool, not voidMarkus Armbruster2-4/+4
See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-32-armbru@redhat.com>
2020-07-10qom: Make functions taking Error ** return bool, not 0/-1Markus Armbruster2-24/+18
Just for consistency. Also fix the example in object_set_props()'s documentation. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-31-armbru@redhat.com>
2020-07-10qom: Use returned bool to check for failure, manual partMarkus Armbruster3-17/+9
The previous commit used Coccinelle to convert from checking the Error object to checking the return value. Convert a few more manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-30-armbru@redhat.com>
2020-07-10qom: Use returned bool to check for failure, Coccinelle partMarkus Armbruster28-142/+96
The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., errp)) { ... } for QOM functions that now return true / false on success / error. Coccinelle script: @@ identifier fun = { object_apply_global_props, object_initialize_child_with_props, object_initialize_child_with_propsv, object_property_get, object_property_get_bool, object_property_parse, object_property_set, object_property_set_bool, object_property_set_int, object_property_set_link, object_property_set_qobject, object_property_set_str, object_property_set_uint, object_set_props, object_set_propv, user_creatable_add_dict, user_creatable_complete, user_creatable_del }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-29-armbru@redhat.com>
2020-07-10qom: Make functions taking Error ** return bool, not voidMarkus Armbruster6-62/+122
See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-28-armbru@redhat.com>
2020-07-10qom: Put name parameter before value / visitor parameterMarkus Armbruster138-741/+712
The object_property_set_FOO() setters take property name and value in an unusual order: void object_property_set_FOO(Object *obj, FOO_TYPE value, const char *name, Error **errp) Having to pass value before name feels grating. Swap them. Same for object_property_set(), object_property_get(), and object_property_parse(). Convert callers with this Coccinelle script: @@ identifier fun = { object_property_get, object_property_parse, object_property_set_str, object_property_set_link, object_property_set_bool, object_property_set_int, object_property_set_uint, object_property_set, object_property_set_qobject }; expression obj, v, name, errp; @@ - fun(obj, v, name, errp) + fun(obj, name, v, errp) Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Convert that one manually. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused by RXCPU being used both as typedef and function-like macro there. Convert manually. The other files using RXCPU that way don't need conversion. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-27-armbru@redhat.com> [Straightforwad conflict with commit 2336172d9b "audio: set default value for pcspk.iobase property" resolved]
2020-07-10qom: Use return values to check for error where that's simplerMarkus Armbruster1-11/+14
When using the Error object to check for error, we need to receive it into a local variable, then propagate() it to @errp. Using the return value permits allows receiving it straight to @errp. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-26-armbru@redhat.com>
2020-07-10qom: Don't handle impossible object_property_get_link() failureMarkus Armbruster9-79/+12
Don't handle object_property_get_link() failure that can't happen unless the programmer screwed up, pass &error_abort. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200707160613.848843-25-armbru@redhat.com>
2020-07-10qom: Crash more nicely on object_property_get_link() failureMarkus Armbruster5-7/+10
Pass &error_abort instead of NULL where the returned value is dereferenced or asserted to be non-null. Drop a now redundant assertion. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-24-armbru@redhat.com>
2020-07-10qom: Rename qdev_get_type() to object_get_type()Markus Armbruster1-2/+2
Commit 2f262e06f0 lifted qdev_get_type() from qdev to object without renaming it accordingly. Do that now. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-23-armbru@redhat.com>
2020-07-10qom: Use error_reportf_err() instead of g_printerr() in examplesMarkus Armbruster1-4/+2
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-22-armbru@redhat.com>
2020-07-10s390x/pci: Fix harmless mistake in zpci's property fid's setterMarkus Armbruster1-1/+3
s390_pci_set_fid() sets zpci->fid_defined to true even when visit_type_uint32() failed. Reproducer: "-device zpci,fid=junk". Harmless in practice, because qdev_device_add() then fails, throwing away @zpci. Fix it anyway. Cc: Matthew Rosato <mjrosato@linux.ibm.com> Cc: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Message-Id: <20200707160613.848843-21-armbru@redhat.com>
2020-07-10qapi: Use returned bool to check for failure, manual partMarkus Armbruster9-56/+44
The previous commit used Coccinelle to convert from checking the Error object to checking the return value. Convert a few more manually. Also tweak control flow in places to conform to the conventional "if error bail out" pattern. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-20-armbru@redhat.com>
2020-07-10qapi: Use returned bool to check for failure, Coccinelle partMarkus Armbruster46-188/+97
The previous commit enables conversion of visit_foo(..., &err); if (err) { ... } to if (!visit_foo(..., errp)) { ... } for visitor functions that now return true / false on success / error. Coccinelle script: @@ identifier fun =~ "check_list|input_type_enum|lv_start_struct|lv_type_bool|lv_type_int64|lv_type_str|lv_type_uint64|output_type_enum|parse_type_bool|parse_type_int64|parse_type_null|parse_type_number|parse_type_size|parse_type_str|parse_type_uint64|print_type_bool|print_type_int64|print_type_null|print_type_number|print_type_size|print_type_str|print_type_uint64|qapi_clone_start_alternate|qapi_clone_start_list|qapi_clone_start_struct|qapi_clone_type_bool|qapi_clone_type_int64|qapi_clone_type_null|qapi_clone_type_number|qapi_clone_type_str|qapi_clone_type_uint64|qapi_dealloc_start_list|qapi_dealloc_start_struct|qapi_dealloc_type_anything|qapi_dealloc_type_bool|qapi_dealloc_type_int64|qapi_dealloc_type_null|qapi_dealloc_type_number|qapi_dealloc_type_str|qapi_dealloc_type_uint64|qobject_input_check_list|qobject_input_check_struct|qobject_input_start_alternate|qobject_input_start_list|qobject_input_start_struct|qobject_input_type_any|qobject_input_type_bool|qobject_input_type_bool_keyval|qobject_input_type_int64|qobject_input_type_int64_keyval|qobject_input_type_null|qobject_input_type_number|qobject_input_type_number_keyval|qobject_input_type_size_keyval|qobject_input_type_str|qobject_input_type_str_keyval|qobject_input_type_uint64|qobject_input_type_uint64_keyval|qobject_output_start_list|qobject_output_start_struct|qobject_output_type_any|qobject_output_type_bool|qobject_output_type_int64|qobject_output_type_null|qobject_output_type_number|qobject_output_type_str|qobject_output_type_uint64|start_list|visit_check_list|visit_check_struct|visit_start_alternate|visit_start_list|visit_start_struct|visit_type_.*"; expression list args; typedef Error; Error *err; @@ - fun(args, &err); - if (err) + if (!fun(args, &err)) { ... } A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-19-armbru@redhat.com>
2020-07-10qapi: Make visitor functions taking Error ** return bool, not voidMarkus Armbruster14-349/+444
See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-18-armbru@redhat.com>
2020-07-10hmp: Eliminate a variable in hmp_migrate_set_parameter()Markus Armbruster1-6/+2
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-17-armbru@redhat.com>
2020-07-10block: Avoid error accumulation in bdrv_img_create()Markus Armbruster1-2/+2
When creating an image fails because the format doesn't support option "backing_file" or "backing_fmt", bdrv_img_create() first has qemu_opt_set() put a generic error into @local_err, then puts the real error into @errp with error_setg(), and then propagates the former to the latter, which throws away the generic error. A bit complicated, but works. Now that qemu_opt_set() returns a useful value, we can simply ignore the generic error instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-16-armbru@redhat.com>
2020-07-10qemu-option: Use returned bool to check for failureMarkus Armbruster32-133/+71
The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for QemuOpts functions that now return true / false on success / error. Coccinelle script: @@ identifier fun = { opts_do_parse, parse_option_bool, parse_option_number, parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set, qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict, qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set, qemu_opts_validate }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-15-armbru@redhat.com> [Conflict with commit 0b6786a9c1 "block/amend: refactor qcow2 amend options" resolved by rerunning Coccinelle on master's version]
2020-07-10qemu-option: Make functions taking Error ** return bool, not voidMarkus Armbruster3-49/+64
See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-14-armbru@redhat.com>
2020-07-10qemu-option: Replace opt_set() by cleaner opt_validate()Markus Armbruster1-16/+19
opt_set() frees its argument @value on failure. Slightly unclean; functions ideally do nothing on failure. To tidy this up, move opt_create() from opt_set() into its callers, along with the cleanup. Rename opt_set() to opt_validate(), noting its similarity to qemu_opts_validate(). Drop redundant parameter @opts; use opt->opts instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-13-armbru@redhat.com>
2020-07-10qemu-option: Factor out helper opt_create()Markus Armbruster1-9/+18
There is just one use so far. The next commit will add more. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-12-armbru@redhat.com>
2020-07-10qemu-option: Simplify around find_default_by_name()Markus Armbruster1-13/+5
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-11-armbru@redhat.com> Reviewed-by: Greg Kurz <groug@kaod.org>
2020-07-10qemu-option: Factor out helper find_default_by_name()Markus Armbruster1-20/+27
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-10-armbru@redhat.com>
2020-07-10qemu-option: Make uses of find_desc_by_name() more similarMarkus Armbruster1-14/+18
This is to make the next commit easier to review. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-9-armbru@redhat.com>
2020-07-10qemu-option: Check return value instead of @err where convenientMarkus Armbruster5-22/+14
Convert uses like opts = qemu_opts_create(..., &err); if (err) { ... } to opts = qemu_opts_create(..., errp); if (!opts) { ... } Eliminate error_propagate() that are now unnecessary. Delete @err that are now unused. Note that we can't drop parallels_open()'s error_propagate() here. We continue to execute it even in the converted case. It's a no-op then: local_err is null. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-8-armbru@redhat.com>
2020-07-10virtio-crypto-pci: Tidy up virtio_crypto_pci_realize()Markus Armbruster1-1/+3
virtio_crypto_pci_realize() continues after realization of its "virtio-crypto-device" fails. Only an object_property_set_link() follows; looks harmless to me. Tidy up anyway: return after failure, just like virtio_rng_pci_realize() does. Cc: "Gonglei (Arei)" <arei.gonglei@huawei.com> Cc: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Gonglei < arei.gonglei@huawei.com> Message-Id: <20200707160613.848843-7-armbru@redhat.com>
2020-07-10macio: Tidy up error handling in macio_newworld_realize()Markus Armbruster1-1/+3
macio_newworld_realize() effectively ignores ns->gpio realization errors, leaking the Error object. Fortunately, macio_gpio_realize() can't actually fail. Tidy up. Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Cc: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-6-armbru@redhat.com>
2020-07-10qdev: Use returned bool to check for qdev_realize() etc. failureMarkus Armbruster65-495/+248
Convert foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their wrappers isa_realize_and_unref(), pci_realize_and_unref(), sysbus_realize(), sysbus_realize_and_unref(), usb_realize_and_unref(). Coccinelle script: @@ identifier fun = { isa_realize_and_unref, pci_realize_and_unref, qbus_realize, qdev_realize, qdev_realize_and_unref, sysbus_realize, sysbus_realize_and_unref, usb_realize_and_unref }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Nothing to convert there; skipped. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-5-armbru@redhat.com>
2020-07-10error: Document Error API usage rulesMarkus Armbruster1-6/+46
This merely codifies existing practice, with one exception: the rule advising against returning void, where existing practice is mixed. When the Error API was created, we adopted the (unwritten) rule to return void when the function returns no useful value on success, unlike GError, which recommends to return true on success and false on error then. When a function returns a distinct error value, say false, a checked call that passes the error up looks like if (!frobnicate(..., errp)) { handle the error... } When it returns void, we need Error *err = NULL; frobnicate(..., &err); if (err) { handle the error... error_propagate(errp, err); } Not only is this more verbose, it also creates an Error object even when @errp is null, &error_abort or &error_fatal. People got tired of the additional boilerplate, and started to ignore the unwritten rule. The result is confusion among developers about the preferred usage. Make the rule advising against returning void official by putting it in writing. This will hopefully reduce confusion. Update the examples accordingly. The remainder of this series will update a substantial amount of code to honor the rule. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-4-armbru@redhat.com> [Tweak prose as per advice from Eric]
2020-07-10error: Improve error.h's big commentMarkus Armbruster1-15/+36
Add headlines to the big comment. Explain examples for NULL, &error_abort and &error_fatal argument better. Tweak rationale for error_propagate_prepend(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707160613.848843-3-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Greg Kurz <groug@kaod.org>
generated by cgit v1.1 at 2025-02-05 21:21:31 +0000