Re: [Xen-devel] [RFC v5 025/126] scripts: add coccinelle script to use auto propagated errp
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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC v5 025/126] scripts: add coccinelle script to use auto propagated errp
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
>> ---
>>
>
>> 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 00..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(...)
>> + {
>> + <...
>> +(
>> +- _err
>> ++ errp
>> +|
>> +- local_err
>> ++ *errp
>> +)
>> + ...>
>> + }
>> +
>> +@@
>> +symbol errp;
>> +@@
>> +
>> +- *errp != NULL
>> ++ *errp
>>
>
> Seems to make sense.
>
--
Best regards,
Vladimir
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC v5 025/126] scripts: add coccinelle script to use auto propagated errp
On 10/11/19 11:04 AM, Vladimir Sementsov-Ogievskiy wrote:
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
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 00..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(...)
+ {
+ <...
+(
+-_err
++errp
+|
+-local_err
++*errp
+)
+ ...>
+ }
+
+@@
+symbol errp;
+@@
+
+- *errp != NULL
++ *errp
Seems to make sense.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC v5 025/126] scripts: add coccinelle script to use auto propagated errp
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
---
+@@
+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.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC v5 025/126] scripts: add coccinelle script to use auto propagated errp
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
CC: Gerd Hoffmann
CC: "Gonglei (Arei)"
CC: Eduardo Habkost
CC: Igor Mammedov
CC: Laurent Vivier
CC: Amit Shah
CC: Kevin Wolf
CC: Max Reitz
CC: John Snow
CC: Ari Sundholm
CC: Pavel Dovgalyuk
CC: Paolo Bonzini
CC: Stefan Hajnoczi
CC: Fam Zheng
CC: Stefan Weil
CC: Ronnie Sahlberg
CC: Peter Lieven
CC: Eric Blake
CC: "Denis V. Lunev"
CC: Markus Armbruster
CC: Alberto Garcia
CC: Jason Dillaman
CC: Wen Congyang
CC: Xie Changlong
CC: Liu Yuan
CC: "Richard W.M. Jones"
CC: Jeff Cody
CC: "Marc-André Lureau"
CC: "Daniel P. Berrangé"
CC: Richard Henderson
CC: Greg Kurz
CC: "Michael S. Tsirkin"
CC: Marcel Apfelbaum
CC: Beniamino Galvani
CC: Peter Maydell
CC: "Cédric Le Goater"
CC: Andrew Jeffery
CC: Joel Stanley
CC: Andrew Baumann
CC: "Philippe Mathieu-Daudé"
CC: Antony Pavlov
CC: Jean-Christophe Dubois
CC: Peter Chubb
CC: Subbaraya Sundeep
CC: Eric Auger
CC: Alistair Francis
CC: "Edgar E. Iglesias"
CC: Stefano Stabellini
CC: Anthony Perard
CC: Paul Durrant
CC: Paul Burton
CC: Aleksandar Rikalo
CC: Chris Wulff
CC: Marek Vasut
CC: David Gibson
CC: Cornelia Huck
CC: Halil Pasic
CC: Christian Borntraeger
CC: "Hervé Poussineau"
CC: Xiao Guangrong
CC: Aurelien Jarno
CC: Aleksandar Markovic
CC: Mark Cave-Ayland
CC: Jason Wang
CC: Laszlo Ersek
CC: Yuval Shaia
CC: Palmer Dabbelt
CC: Sagar Karandikar
CC: Bastian Koppelmann
CC: David Hildenbrand
CC: Thomas Huth
CC: Eric Farman
CC: Matthew Rosato
CC: Hannes Reinecke
CC: Michael Walle
CC: Artyom Tarasenko
CC: Stefan Berger
CC: Samuel Thibault
CC: Alex Williamson
CC: Tony Krowiak
CC: Pierre Morel
CC: Michael Roth
CC: Hailiang Zhang
CC: Juan Quintela
CC: "Dr. David Alan Gilbert"
CC: Luigi Rizzo
CC: Giuseppe Lettieri
CC: Vincenzo Maffione
CC: Jan Kiszka
CC: Anthony Green
CC: Stafford Horne
CC: Guan Xuetao
CC: Max Filippov
CC: qemu-bl...@nongnu.org
CC: integrat...@gluster.org
CC: sheep...@lists.wpkg.org
CC: qemu-...@nongnu.org
CC: xen-devel@lists.xenproject.org
CC: qemu-...@nongnu.org
CC: qemu-s3...@nongnu.org
CC: qemu-ri...@nongnu.org
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 00..d9731620aa
--- /dev/null
+++ b/scripts/coccinelle/auto-propagated-errp.cocci
@@ -0,0 +1,118 @@
+@rule0@
+// Add invocation to errp-functions where necessary
+identifier fn, local_err;
+symbol errp;
+@@
+
+ fn(..., Error **errp, ...)
+ {
++ ERRP_AUTO_PROPAGATE();
+<+...
+(
+error_append_hint(errp, ...);
+|
+error_prepend(errp, ...);
+|
+Error *local_err = NULL;
+)
+...+>
+ }
+
+@@
+// Drop doubled invocation
+identifier rule0.fn;
+@@
+
+ fn(...)
+{
+- ERRP_AUTO_PROPAGATE();
+ERRP_AUTO_PROPAGATE();
+...
+}
+
+@rule1@
+// Drop local_err
+identifier fn, local_err;
+symbol errp;
+@@
+
+ fn(..., Error **errp, ...)
+ {
+ <...
+-Error *local_err = NULL;
+ ...>
+ }
+
+@@
+// 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);
+ }
+
+@@
+identifier rule1.fn;
+identifier rule1.local_err;
+@@
+
+ fn(...)
+ {
+ <...
+(
+-error_free(local_err);
+-local_err = NULL;
++error_free_errp(errp);
+|
+-error_free(local_err);
++error_free_errp(errp);
+|
+-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);
+)
+ ...>
+ }
+
+@@
+identifier rule1.fn;
+identifier rule1.local_err;
+@@
+
+ fn(...)
+ {
+ <...
+(
+-_err
++errp
+|
+-local_err
++*errp
+)
+ ...>
+ }
+
+@@
+symbol errp;
+@@
+
+- *errp != NULL
++ *errp
--
2.21.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
