[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp


  • To: Markus Armbruster <armbru@xxxxxxxxxx>
  • From: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx>
  • Date: Wed, 11 Mar 2020 17:46:10 +0300
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=virtuozzo.com; dmarc=pass action=none header.from=virtuozzo.com; dkim=pass header.d=virtuozzo.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=kzSVwZC4T7EmlDuBOo+cCzHoP3zUSdHBA1XijoS2cBs=; b=DVMwxc2zrYXekN3dXIuwYysFPW1khmFwC1MjXlUXlABbT6TbI28zFVbKwFlj8b3sKvKUet8S+/3YEN0e5fINmWKjCopCGmWRfwCTfBeBlgEBQAiRT9+Iah4kcALnTszkTfVP00BMrKusYj6abZrreLQxWPk0YYyjulPf77fqUIatstmHF8Pod5lkAUxwZzByb4HDqLeOyLl4mfJAS9E7BPi0fuKvIWnEJMLvRUZ3v4yBT5M9kpGRMUdf49/wpWeuGrHgtnVXpBaH7D9X40Q01TTDfOASAitLJdK+HSEDYRRudxkXoMtmV9RVfwBEHiLY5IOmM34UboAWB3iXjFbBLw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cly4sW/UavUu6zX/E6LdneGvvGT4X3GRCZxdhMQv5f2t5WqwG2UHz6BBmRDVtmIwq7olCJxgdy+n8GOHxjP1I3LRV7s6CxAtL1MIS3+uIsswYrp+BOjHBXvjESY+Lm3nDbXayd/PT0ygWjbbl5bdTSRndhQULOzvmkpmdxIrTZO8ww8O71mWuaNRI624qBKO9jQPODoCyBgKVGo2Mo/9EYSzfFDqEKvA0WEmBe8moIa7B2HWqeL+uzSwr6trAE6YajH38/0rFa+pg6w49yJOQ2yMkevSKkasjXbVFsB7dhBpxKJ9tTUOiil/0sDd1SIAmltYiboBiuvtggRd1eSAsw==
  • Authentication-results: spf=none (sender IP is ) smtp.mailfrom=vsementsov@xxxxxxxxxxxxx;
  • Cc: Kevin Wolf <kwolf@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, qemu-block@xxxxxxxxxx, Paul Durrant <paul@xxxxxxx>, Laszlo Ersek <lersek@xxxxxxxxxx>, Christian Schoenebeck <qemu_oss@xxxxxxxxxxxxx>, Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>, qemu-devel@xxxxxxxxxx, Greg Kurz <groug@xxxxxxxx>, Gerd Hoffmann <kraxel@xxxxxxxxxx>, Stefan Hajnoczi <stefanha@xxxxxxxxxx>, Anthony Perard <anthony.perard@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Max Reitz <mreitz@xxxxxxxxxx>, Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>, Stefan Berger <stefanb@xxxxxxxxxxxxx>
  • Delivery-date: Wed, 11 Mar 2020 14:46:24 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

11.03.2020 17:41, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes:

11.03.2020 12:38, Markus Armbruster wrote:
Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes:

09.03.2020 12:56, Markus Armbruster wrote:
Suggest

       scripts: Coccinelle script to use auto-propagated errp

or

       scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE()

Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes:
[...]
+// Note, that we update everything related to matched by rule1 function name
+// and local_err name. We may match something not related to the pattern
+// matched by rule1. For example, local_err may be defined with the same name
+// in different blocks inside one function, and in one block follow the
+// propagation pattern and in other block doesn't. Or we may have several
+// functions with the same name (for different configurations).

Context: rule1 matches functions that have all three of

* an Error **errp parameter

* an Error *local_err = NULL variable declaration

* an error_propagate(errp, local_err) or error_propagate_prepend(errp,
     local_err, ...) expression, where @errp is the parameter and
     @local_err is the variable.

If I understand you correctly, you're pointing out two potential issues:

1. This rule can match functions rule1 does not match if there is
another function with the same name that rule1 does match.

2. This rule matches in the entire function matched by rule1, even when
parts of that function use a different @errp or @local_err.

I figure these apply to all rules with identifier rule1.fn, not just
this one.  Correct?

Regarding 1.  There must be a better way to chain rules together, but I
don't know it.

Hmm, what about something like this:

@rule1 disable optional_qualifier exists@
identifier fn, local_err;
symbol errp;
@@

   fn(..., Error **
- errp
+ ___errp_coccinelle_updating___
      , ...)
   {
       ...
       Error *local_err = NULL;
       ...
(
      error_propagate_prepend(errp, local_err, ...);
|
      error_propagate(errp, local_err);
)
       ...
   }


[..]

match symbol ___errp_coccinelle_updating___ in following rules in function 
header

[..]


@ disable optional_qualifier@
identifier fn, local_err;
symbol errp;
@@

   fn(..., Error **
- ___errp_coccinelle_updating___
+ errp
      , ...)
   {
       ...
   }


- hacky, but seems not more hacky than python detection, and should work better

As simple, forceful and unsubtle as a sledgehammer.  I like it :)



Hmm, not so simple.

It leads to reindenting of function header, which is bad.

Because ___errp_coccinelle_updating___ is longer than errp, I guess.
Try ____?

I'm afraid not. It's because it just adds \n, when I do

...,

- errp
+ ___errp_coccinelle_updating___
,...


Possible solution is instead

fn(...)
{
+   ___errp_coccinelle_updating___();


but this slow down coccinelle. For example, on block.c from ~3s to 1m16s.

.

So, I'm returning to just a warning.

I think something simple like

@@
identifier rule1.fn;
position p != rule1.p;
@@

fn@p(...) {...}

@ script:python@

<print warning>

should work.

Up to you.



--
Best regards,
Vladimir

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.