[RFC,v5,025/126] scripts: add coccinelle script to use auto propagated errp

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---

CC: Gerd Hoffmann <kraxel@redhat.com>
CC: "Gonglei (Arei)" <arei.gonglei@huawei.com>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: Igor Mammedov <imammedo@redhat.com>
CC: Laurent Vivier <lvivier@redhat.com>
CC: Amit Shah <amit@kernel.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Ari Sundholm <ari@tuxera.com>
CC: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <fam@euphon.net>
CC: Stefan Weil <sw@weilnetz.de>
CC: Ronnie Sahlberg <ronniesahlberg@gmail.com>
CC: Peter Lieven <pl@kamp.de>
CC: Eric Blake <eblake@redhat.com>
CC: "Denis V. Lunev" <den@openvz.org>
CC: Markus Armbruster <armbru@redhat.com>
CC: Alberto Garcia <berto@igalia.com>
CC: Jason Dillaman <dillaman@redhat.com>
CC: Wen Congyang <wencongyang2@huawei.com>
CC: Xie Changlong <xiechanglong.d@gmail.com>
CC: Liu Yuan <namei.unix@gmail.com>
CC: "Richard W.M. Jones" <rjones@redhat.com>
CC: Jeff Cody <codyprime@gmail.com>
CC: "Marc-André Lureau" <marcandre.lureau@redhat.com>
CC: "Daniel P. Berrangé" <berrange@redhat.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Greg Kurz <groug@kaod.org>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
CC: Beniamino Galvani <b.galvani@gmail.com>
CC: Peter Maydell <peter.maydell@linaro.org>
CC: "Cédric Le Goater" <clg@kaod.org>
CC: Andrew Jeffery <andrew@aj.id.au>
CC: Joel Stanley <joel@jms.id.au>
CC: Andrew Baumann <Andrew.Baumann@microsoft.com>
CC: "Philippe Mathieu-Daudé" <philmd@redhat.com>
CC: Antony Pavlov <antonynpavlov@gmail.com>
CC: Jean-Christophe Dubois <jcd@tribudubois.net>
CC: Peter Chubb <peter.chubb@nicta.com.au>
CC: Subbaraya Sundeep <sundeep.lkml@gmail.com>
CC: Eric Auger <eric.auger@redhat.com>
CC: Alistair Francis <alistair@alistair23.me>
CC: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Anthony Perard <anthony.perard@citrix.com>
CC: Paul Durrant <paul@xen.org>
CC: Paul Burton <pburton@wavecomp.com>
CC: Aleksandar Rikalo <arikalo@wavecomp.com>
CC: Chris Wulff <crwulff@gmail.com>
CC: Marek Vasut <marex@denx.de>
CC: David Gibson <david@gibson.dropbear.id.au>
CC: Cornelia Huck <cohuck@redhat.com>
CC: Halil Pasic <pasic@linux.ibm.com>
CC: Christian Borntraeger <borntraeger@de.ibm.com>
CC: "Hervé Poussineau" <hpoussin@reactos.org>
CC: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
CC: Aurelien Jarno <aurelien@aurel32.net>
CC: Aleksandar Markovic <amarkovic@wavecomp.com>
CC: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
CC: Jason Wang <jasowang@redhat.com>
CC: Laszlo Ersek <lersek@redhat.com>
CC: Yuval Shaia <yuval.shaia@oracle.com>
CC: Palmer Dabbelt <palmer@sifive.com>
CC: Sagar Karandikar <sagark@eecs.berkeley.edu>
CC: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
CC: David Hildenbrand <david@redhat.com>
CC: Thomas Huth <thuth@redhat.com>
CC: Eric Farman <farman@linux.ibm.com>
CC: Matthew Rosato <mjrosato@linux.ibm.com>
CC: Hannes Reinecke <hare@suse.com>
CC: Michael Walle <michael@walle.cc>
CC: Artyom Tarasenko <atar4qemu@gmail.com>
CC: Stefan Berger <stefanb@linux.ibm.com>
CC: Samuel Thibault <samuel.thibault@ens-lyon.org>
CC: Alex Williamson <alex.williamson@redhat.com>
CC: Tony Krowiak <akrowiak@linux.ibm.com>
CC: Pierre Morel <pmorel@linux.ibm.com>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
CC: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
CC: Juan Quintela <quintela@redhat.com>
CC: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
CC: Luigi Rizzo <rizzo@iet.unipi.it>
CC: Giuseppe Lettieri <g.lettieri@iet.unipi.it>
CC: Vincenzo Maffione <v.maffione@gmail.com>
CC: Jan Kiszka <jan.kiszka@siemens.com>
CC: Anthony Green <green@moxielogic.com>
CC: Stafford Horne <shorne@gmail.com>
CC: Guan Xuetao <gxt@mprc.pku.edu.cn>
CC: Max Filippov <jcmvbkbc@gmail.com>
CC: qemu-block@nongnu.org
CC: integration@gluster.org
CC: sheepdog@lists.wpkg.org
CC: qemu-arm@nongnu.org
CC: xen-devel@lists.xenproject.org
CC: qemu-ppc@nongnu.org
CC: qemu-s390x@nongnu.org
CC: qemu-riscv@nongnu.org

 scripts/coccinelle/auto-propagated-errp.cocci | 118 ++++++++++++++++++
 1 file changed, 118 insertions(+)
 create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci

On 10/11/19 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 

>   scripts/coccinelle/auto-propagated-errp.cocci | 118 ++++++++++++++++++
>   1 file changed, 118 insertions(+)
>   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
> 
> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci b/scripts/coccinelle/auto-propagated-errp.cocci
> new file mode 100644
> index 0000000000..d9731620aa
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci

> +@rule1@
> +// Drop local_err
> +identifier fn, local_err;
> +symbol errp;
> +@@
> +
> + fn(..., Error **errp, ...)
> + {
> +     <...
> +-    Error *local_err = NULL;
> +     ...>
> + }
> +

So our goal is to automate removal of all local_err (including when it 
is spelled err)...

> +@@
> +// Handle pattern with goto, otherwise we'll finish up
> +// with labels at function end which will not compile.
> +identifier rule1.fn;
> +identifier rule1.local_err;
> +identifier OUT;
> +@@
> +
> + fn(...)
> + {
> +     <...
> +-    goto OUT;
> ++    return;
> +     ...>
> +- OUT:
> +-    error_propagate(errp, local_err);
> + }
> +

this dangling label cleanup makes sense

> +@@
> +identifier rule1.fn;
> +identifier rule1.local_err;
> +@@
> +
> + fn(...)
> + {
> +     <...
> +(
> +-    error_free(local_err);
> +-    local_err = NULL;
> ++    error_free_errp(errp);

This does not make sense - error_free_errp() is not defined prior to 
this series or anywhere in patches 1-24, if I'm reading it correctly.

> +|
> +-    error_free(local_err);
> ++    error_free_errp(errp);

and again

> +|
> +-    error_report_err(local_err);
> ++    error_report_errp(errp);
> +|
> +-    warn_report_err(local_err);
> ++    warn_report_errp(errp);
> +|
> +-    error_propagate_prepend(errp, local_err,
> ++    error_prepend(errp,
> +                              ...);
> +|
> +-    error_propagate(errp, local_err);
> +)
> +     ...>
> + }
> +

It looks like once this script is run, error_propagate_prepend() will 
have no clients.  Is there a non-generated cleanup patch that removes it 
(and once it is removed, it can also be removed from the .cocci script 
as no further clients will reappear later)?


> +@@
> +identifier rule1.fn;
> +identifier rule1.local_err;
> +@@
> +
> + fn(...)
> + {
> +     <...
> +(
> +-    &local_err
> ++    errp
> +|
> +-    local_err
> ++    *errp
> +)
> +     ...>
> + }
> +
> +@@
> +symbol errp;
> +@@
> +
> +- *errp != NULL
> ++ *errp
> 

Seems to make sense.

On 10/11/19 12:12 PM, Eric Blake wrote:
> On 10/11/19 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>

>> +@@
>> +identifier rule1.fn;
>> +identifier rule1.local_err;
>> +@@
>> +
>> + fn(...)
>> + {
>> +     <...
>> +(
>> +-    error_free(local_err);
>> +-    local_err = NULL;
>> ++    error_free_errp(errp);
> 
> This does not make sense - error_free_errp() is not defined prior to 
> this series or anywhere in patches 1-24, if I'm reading it correctly.

Apologies - I sent my reply to this message before 21/126 showed up in 
my inbox, and didn't realize that I had skipped over sequencing.  This 
part is fine after all, but it points to the perils of reviewing a 
thread as it comes in piecemeal, vs. reviewing an actual branch with all 
patches instantly available.

11.10.2019 20:12, Eric Blake wrote:
> On 10/11/19 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>
> 
>>   scripts/coccinelle/auto-propagated-errp.cocci | 118 ++++++++++++++++++
>>   1 file changed, 118 insertions(+)
>>   create mode 100644 scripts/coccinelle/auto-propagated-errp.cocci
>>
>> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci b/scripts/coccinelle/auto-propagated-errp.cocci
>> new file mode 100644
>> index 0000000000..d9731620aa
>> --- /dev/null
>> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
> 
>> +@rule1@
>> +// Drop local_err
>> +identifier fn, local_err;
>> +symbol errp;
>> +@@
>> +
>> + fn(..., Error **errp, ...)
>> + {
>> +     <...
>> +-    Error *local_err = NULL;
>> +     ...>
>> + }
>> +
> 
> So our goal is to automate removal of all local_err (including when it is spelled err)...
> 
>> +@@
>> +// Handle pattern with goto, otherwise we'll finish up
>> +// with labels at function end which will not compile.
>> +identifier rule1.fn;
>> +identifier rule1.local_err;
>> +identifier OUT;
>> +@@
>> +
>> + fn(...)
>> + {
>> +     <...
>> +-    goto OUT;
>> ++    return;
>> +     ...>
>> +- OUT:
>> +-    error_propagate(errp, local_err);
>> + }
>> +
> 
> this dangling label cleanup makes sense
> 
>> +@@
>> +identifier rule1.fn;
>> +identifier rule1.local_err;
>> +@@
>> +
>> + fn(...)
>> + {
>> +     <...
>> +(
>> +-    error_free(local_err);
>> +-    local_err = NULL;
>> ++    error_free_errp(errp);
> 
> This does not make sense - error_free_errp() is not defined prior to this series or anywhere in patches 1-24, if I'm reading it correctly.
> 
>> +|
>> +-    error_free(local_err);
>> ++    error_free_errp(errp);
> 
> and again
> 
>> +|
>> +-    error_report_err(local_err);
>> ++    error_report_errp(errp);
>> +|
>> +-    warn_report_err(local_err);
>> ++    warn_report_errp(errp);
>> +|
>> +-    error_propagate_prepend(errp, local_err,
>> ++    error_prepend(errp,
>> +                              ...);
>> +|
>> +-    error_propagate(errp, local_err);
>> +)
>> +     ...>
>> + }
>> +
> 
> It looks like once this script is run, error_propagate_prepend() will have no clients.

No, it still have a bit, when working with error_copy, and/or moving errors from/to structures.

> Is there a non-generated cleanup patch that removes it (and once it is removed, it can also be removed from the .cocci script as no further clients will reappear later)?

Maybe.

> 
> 
>> +@@
>> +identifier rule1.fn;
>> +identifier rule1.local_err;
>> +@@
>> +
>> + fn(...)
>> + {
>> +     <...
>> +(
>> +-    &local_err
>> ++    errp
>> +|
>> +-    local_err
>> ++    *errp
>> +)
>> +     ...>
>> + }
>> +
>> +@@
>> +symbol errp;
>> +@@
>> +
>> +- *errp != NULL
>> ++ *errp
>>
> 
> Seems to make sense.
>

On 10/14/19 3:19 AM, Vladimir Sementsov-Ogievskiy wrote:

>>> +|
>>> +-    error_propagate(errp, local_err);
>>> +)
>>> +     ...>
>>> + }
>>> +
>>
>> It looks like once this script is run, error_propagate_prepend() will have no clients.
> 
> No, it still have a bit, when working with error_copy, and/or moving errors from/to structures.

No public clients. So can we turn it into a static function only used by 
error.c?

> 
>> Is there a non-generated cleanup patch that removes it (and once it is removed, it can also be removed from the .cocci script as no further clients will reappear later)?
> 
> Maybe.
> 

Basically, if we can get error_propagate out of error.h, that's a good 
sign.  But it's not a show-stopper if we can't do it for some legitimate 
reason (such a reason might include that the definition of the 
ERRP_AUTO_PROPAGATE macro is easier to write if error_propagate remains 
in the .h), so we'll just have to see what is possible.