Eric Blake <ebl...@redhat.com> writes:
> On 6/24/20 11:43 AM, Markus Armbruster wrote:
>> Replace
>>
>> error_setg(&err, ...);
>> error_propagate(errp, err);
>>
>> by
>>
>> error_setg(errp, ...);
>>
>> Related pattern:
>
> Nice explanation.
>
>> 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 <arm...@redhat.com>
>> ---
>> backends/cryptodev.c | 11 +++---
>> backends/hostmem-file.c | 19 +++-------
>> backends/hostmem-memfd.c | 15 ++++----
>> backends/hostmem.c | 27 ++++++--------
>> block/throttle-groups.c | 22 +++++------
>> hw/hyperv/vmbus.c | 5 +--
>> hw/i386/pc.c | 35 ++++++------------
>> hw/mem/nvdimm.c | 17 ++++-----
>> hw/mem/pc-dimm.c | 14 +++----
>> hw/misc/aspeed_sdmc.c | 3 +-
>> hw/ppc/rs6000_mc.c | 9 ++---
>> hw/ppc/spapr.c | 73 ++++++++++++++++---------------------
>> hw/ppc/spapr_pci.c | 14 +++----
>> hw/s390x/ipl.c | 23 +++++-------
>> hw/s390x/sclp.c | 12 ++----
>> hw/xen/xen_pt_config_init.c | 3 +-
>> iothread.c | 12 +++---
>> net/colo-compare.c | 20 ++++------
>> net/dump.c | 10 ++---
>> net/filter-buffer.c | 10 ++---
>> qga/commands-win32.c | 16 +++-----
>> 21 files changed, 151 insertions(+), 219 deletions(-)
>
> A bit bigger, and starts to be too complex to ask Coccinelle to
> directly fix it (but at least using it for identification is nice).
> But the patch is still manageable, and hopefully not too many
> instances creep back in during the meantime while waiting for this
> series to land.
>
>> @@ -140,7 +138,6 @@ static void file_memory_backend_set_pmem(Object *o, bool
>> value, Error **errp)
>> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
>> if (host_memory_backend_mr_inited(backend)) {
>> -
>> error_setg(errp, "cannot change property 'pmem' of %s.",
>> object_get_typename(o));
>> return;
>
> Unrelated cleanup. Does it belong in a different patch?
I don't have a better one at hand. The only other patch touching this
file is a worse choice: PATCH 23.
>> @@ -148,13 +145,9 @@ static void file_memory_backend_set_pmem(Object *o,
>> bool value, Error **errp)
>> #ifndef CONFIG_LIBPMEM
>> if (value) {
>> - Error *local_err = NULL;
>> -
>> - error_setg(&local_err,
>> - "Lack of libpmem support while setting the 'pmem=on'"
>> + error_setg(errp, "Lack of libpmem support while setting the
>> 'pmem=on'"
>> " of %s. We can't ensure data persistence.",
>
> Pre-existing - doesn't follow our usual error message content
> conventions regarding trailing '.'.
Yes. But this patch feels too big for me to also improve error
messages. I can do it separately.
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -1517,15 +1517,16 @@ static void spapr_pci_plug(HotplugHandler
>> *plug_handler,
>> */
>> if (plugged_dev->hotplugged && bus->devices[PCI_DEVFN(slotnr, 0)] &&
>> PCI_FUNC(pdev->devfn) != 0) {
>> - error_setg(&local_err, "PCI: slot %d function 0 already ocuppied by
>> %s,"
>> + error_setg(errp, "PCI: slot %d function 0 already ocuppied by %s,"
>> " additional functions can no longer be exposed to
>> guest.",
>
> Another one. Also, s/ocuppied/occupied/ while touching it.
Likewise.
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
