[Qemu-devel] [PATCH 0/2] qobject coccinelle followups

[Qemu-devel] [PATCH 0/2] qobject coccinelle followups

[Eric Blake]

Inspired by https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg02505.html

Surprisingly, the updated script didn't find any existing longhand
use as candidates for QDECREF() (our last such use was cleaned up
as a side effect, in commit b72fe9e), but it DID find another straggler
missed in commit 579cf1d1.

Eric Blake (2):
  qobject: Update coccinelle script to catch Q{INC,DEC}REF
  qobject: Catch another straggler for use of qdict_put_str()

 qemu-img.c                       | 2 +-
 scripts/coccinelle/qobject.cocci | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.9.4

[Qemu-devel] [PATCH 1/2] qobject: Update coccinelle script to catch Q{INC, DEC}REF

[Eric Blake]

A recent patch submission was about to use qobject_decref(QOBJECT(E)),
even though we already have QDECREF(E) for that purpose.  While our
tree is currently free from the longhand form, we might as well update
our coccinelle script to catch any future relapses.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/coccinelle/qobject.cocci | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/coccinelle/qobject.cocci b/scripts/coccinelle/qobject.cocci
index 97703a4..656dc3e 100644
--- a/scripts/coccinelle/qobject.cocci
+++ b/scripts/coccinelle/qobject.cocci
@@ -3,6 +3,12 @@
 expression Obj, Key, E;
 @@
 (
+- qobject_incref(QOBJECT(E));
++ QINCREF(E);
+|
+- qobject_decref(QOBJECT(E));
++ QDECREF(E);
+|
 - qdict_put_obj(Obj, Key, QOBJECT(E));
 + qdict_put(Obj, Key, E);
 |
-- 
2.9.4

[Qemu-devel] [PATCH 2/2] qobject: Catch another straggler for use of qdict_put_str()

[Eric Blake]

Dan's addition of key-secret improvements in commit 29cf9336 was
developed prior to the addition of QDict scalar insertion macros,
but merged after the general cleanup in commit 46f5ac20.
Patch created mechanically by rerunning:
  spatch --sp-file scripts/coccinelle/qobject.cocci \
         --macro-file scripts/cocci-macro-file.h --dir . --in-place

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0ad698d..6c972f7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -355,7 +355,7 @@ static int img_add_key_secrets(void *opaque,
     QDict *options = opaque;

     if (g_str_has_suffix(name, "key-secret")) {
-        qdict_put(options, name, qstring_from_str(value));
+        qdict_put_str(options, name, value);
     }

     return 0;
-- 
2.9.4

Re: [Qemu-devel] [PATCH 1/2] qobject: Update coccinelle script to catch Q{INC, DEC}REF

[Marc-André Lureau]

Hi

On Fri, Jun 9, 2017 at 7:20 PM Eric Blake <eblake@redhat.com> wrote:

> A recent patch submission was about to use qobject_decref(QOBJECT(E)),
> even though we already have QDECREF(E) for that purpose.  While our
> tree is currently free from the longhand form, we might as well update
>

Oh?

 $ git grep 'object_unref(OBJECT('  | wc -l
152

our coccinelle script to catch any future relapses.
>

sadly, coccinelle is unabarebly slow on my machine, not easy to grasp, and
thereby fails to catch a lot of cases.. I am looking at alternative from
clang tools/lib, by curiosity.


>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  scripts/coccinelle/qobject.cocci | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/scripts/coccinelle/qobject.cocci
> b/scripts/coccinelle/qobject.cocci
> index 97703a4..656dc3e 100644
> --- a/scripts/coccinelle/qobject.cocci
> +++ b/scripts/coccinelle/qobject.cocci
> @@ -3,6 +3,12 @@
>  expression Obj, Key, E;
>  @@
>  (
> +- qobject_incref(QOBJECT(E));
> ++ QINCREF(E);
> +|
> +- qobject_decref(QOBJECT(E));
> ++ QDECREF(E);
> +|
>  - qdict_put_obj(Obj, Key, QOBJECT(E));
>  + qdict_put(Obj, Key, E);
>  |
> --
> 2.9.4
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/2] qobject: Update coccinelle script to catch Q{INC, DEC}REF

[Marc-André Lureau]

On Fri, Jun 9, 2017 at 7:28 PM Marc-André Lureau <marcandre.lureau@gmail.com>
wrote:

> Hi
>
> On Fri, Jun 9, 2017 at 7:20 PM Eric Blake <eblake@redhat.com> wrote:
>
>> A recent patch submission was about to use qobject_decref(QOBJECT(E)),
>> even though we already have QDECREF(E) for that purpose.  While our
>> tree is currently free from the longhand form, we might as well update
>>
>
> Oh?
>
>  $ git grep 'object_unref(OBJECT('  | wc -l
> 152
>
>
ah not the same object kind :)

our coccinelle script to catch any future relapses.
>>
>
> sadly, coccinelle is unabarebly slow on my machine, not easy to grasp, and
> thereby fails to catch a lot of cases.. I am looking at alternative from
> clang tools/lib, by curiosity.
>
>
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  scripts/coccinelle/qobject.cocci | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/scripts/coccinelle/qobject.cocci
>> b/scripts/coccinelle/qobject.cocci
>> index 97703a4..656dc3e 100644
>> --- a/scripts/coccinelle/qobject.cocci
>> +++ b/scripts/coccinelle/qobject.cocci
>> @@ -3,6 +3,12 @@
>>  expression Obj, Key, E;
>>  @@
>>  (
>> +- qobject_incref(QOBJECT(E));
>> ++ QINCREF(E);
>> +|
>> +- qobject_decref(QOBJECT(E));
>> ++ QDECREF(E);
>> +|
>>  - qdict_put_obj(Obj, Key, QOBJECT(E));
>>  + qdict_put(Obj, Key, E);
>>  |
>> --
>> 2.9.4
>>
>>
>> --
> Marc-André Lureau
>
-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/2] qobject: Update coccinelle script to catch Q{INC, DEC}REF

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

On 06/09/2017 10:28 AM, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Jun 9, 2017 at 7:20 PM Eric Blake <eblake@redhat.com> wrote:
> 
>> A recent patch submission was about to use qobject_decref(QOBJECT(E)),
>> even though we already have QDECREF(E) for that purpose.  While our
>> tree is currently free from the longhand form, we might as well update
>>
> 
> Oh?
> 
>  $ git grep 'object_unref(OBJECT('  | wc -l
> 152

Quit mixing QOM and QObject ;)

$ git grep qobject_'..cref(QOBJECT'
include/qapi/qmp/qobject.h:    qobject_incref(QOBJECT(obj))
scripts/coccinelle/qobject.cocci:- qobject_incref(QOBJECT(E));
scripts/coccinelle/qobject.cocci:- qobject_decref(QOBJECT(E));

Yes, we may want to add a script to clean QOM abuses, but that's a
different project for a different day.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/2] qobject: Update coccinelle script to catch Q{INC, DEC}REF

[Marc-André Lureau]

On Fri, Jun 9, 2017 at 7:33 PM Eric Blake <eblake@redhat.com> wrote:

> On 06/09/2017 10:28 AM, Marc-André Lureau wrote:
> > Hi
> >
> > On Fri, Jun 9, 2017 at 7:20 PM Eric Blake <eblake@redhat.com> wrote:
> >
> >> A recent patch submission was about to use qobject_decref(QOBJECT(E)),
> >> even though we already have QDECREF(E) for that purpose.  While our
> >> tree is currently free from the longhand form, we might as well update
> >>
> >
> > Oh?
> >
> >  $ git grep 'object_unref(OBJECT('  | wc -l
> > 152
>
> Quit mixing QOM and QObject ;)
>
> $ git grep qobject_'..cref(QOBJECT'
> include/qapi/qmp/qobject.h:    qobject_incref(QOBJECT(obj))
> scripts/coccinelle/qobject.cocci:- qobject_incref(QOBJECT(E));
> scripts/coccinelle/qobject.cocci:- qobject_decref(QOBJECT(E));
>
>
>
Yeah, I got mixed by your comments, my patch doesn't do
qobject_decref(QOBJECT(E)) but object_unref(OBJECT(E)), which doesn't have
macro helper afaik.


> Yes, we may want to add a script to clean QOM abuses, but that's a
> different project for a different day.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266 <+1%20919-301-3266>
> Virtualization:  qemu.org | libvirt.org
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/2] qobject: Update coccinelle script to catch Q{INC, DEC}REF

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]

On 06/09/2017 10:28 AM, Marc-André Lureau wrote:

> sadly, coccinelle is unabarebly slow on my machine,

Odd.  It's pretty fast for this series:

$ time spatch --sp-file scripts/coccinelle/qobject.cocci \
  --macro-file scripts/cocci-macro-file.h --dir . --in-place 2>/dev/null

real	0m2.136s
user	0m2.003s
sys	0m0.125s

This is on Fedora 25.  Although I _will_ grant that the time taken is
also dependent on the complexity of the script it is running on (some
scripts are inherently slower than others, and coccinelle takes
shortcuts like grepping for which files to even fully analyze, but not
all scripts are conducive to those shortcuts).

> not easy to grasp, and
> thereby fails to catch a lot of cases..

I agree with the 'not easy to grasp'. As for not catching cases, you
HAVE to use the --macro-file scripts/cocci-macro-file.h to avoid a lot
of skipped files.  But even so, you're probably right that it still
misses some things (although it often catches far more than "grep in
anger" is able to do).

> I am looking at alternative from
> clang tools/lib, by curiosity.

I'll be interested to see if it produces any (easy-to-reproduce) cleanup
results.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/2] qobject: Catch another straggler for use of qdict_put_str()

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]

On 2017-06-09 17:20, Eric Blake wrote:
> Dan's addition of key-secret improvements in commit 29cf9336 was
> developed prior to the addition of QDict scalar insertion macros,
> but merged after the general cleanup in commit 46f5ac20.
> Patch created mechanically by rerunning:
>   spatch --sp-file scripts/coccinelle/qobject.cocci \
>          --macro-file scripts/cocci-macro-file.h --dir . --in-place
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] qobject: Catch another straggler for use of qdict_put_str()

[Alberto Garcia]

On Fri 09 Jun 2017 05:20:17 PM CEST, Eric Blake <eblake@redhat.com> wrote:
> Dan's addition of key-secret improvements in commit 29cf9336 was
> developed prior to the addition of QDict scalar insertion macros,
> but merged after the general cleanup in commit 46f5ac20.
> Patch created mechanically by rerunning:
>   spatch --sp-file scripts/coccinelle/qobject.cocci \
>          --macro-file scripts/cocci-macro-file.h --dir . --in-place
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH 0/2] qobject coccinelle followups

[Michael Tokarev]

09.06.2017 18:20, Eric Blake wrote:
> Inspired by https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg02505.html
> 
> Surprisingly, the updated script didn't find any existing longhand
> use as candidates for QDECREF() (our last such use was cleaned up
> as a side effect, in commit b72fe9e), but it DID find another straggler
> missed in commit 579cf1d1.
> 
> Eric Blake (2):
>   qobject: Update coccinelle script to catch Q{INC,DEC}REF
>   qobject: Catch another straggler for use of qdict_put_str()

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH 0/2] qobject coccinelle followups

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

On 07/12/2017 08:35 AM, Michael Tokarev wrote:
> 09.06.2017 18:20, Eric Blake wrote:
>> Inspired by https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg02505.html
>>
>> Surprisingly, the updated script didn't find any existing longhand
>> use as candidates for QDECREF() (our last such use was cleaned up
>> as a side effect, in commit b72fe9e), but it DID find another straggler
>> missed in commit 579cf1d1.
>>
>> Eric Blake (2):
>>   qobject: Update coccinelle script to catch Q{INC,DEC}REF
>>   qobject: Catch another straggler for use of qdict_put_str()
> 
> Applied to -trivial, thanks!

Markus has taken v2 of these patches through his qapi-next tree:
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05589.html

so you can drop this v1 from -trivial

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 0/2] qobject coccinelle followups

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 07/12/2017 08:35 AM, Michael Tokarev wrote:
>> 09.06.2017 18:20, Eric Blake wrote:
>>> Inspired by https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg02505.html
>>>
>>> Surprisingly, the updated script didn't find any existing longhand
>>> use as candidates for QDECREF() (our last such use was cleaned up
>>> as a side effect, in commit b72fe9e), but it DID find another straggler
>>> missed in commit 579cf1d1.
>>>
>>> Eric Blake (2):
>>>   qobject: Update coccinelle script to catch Q{INC,DEC}REF
>>>   qobject: Catch another straggler for use of qdict_put_str()
>> 
>> Applied to -trivial, thanks!
>
> Markus has taken v2 of these patches through his qapi-next tree:
> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg05589.html
>
> so you can drop this v1 from -trivial

I've dragged my feet on the pull request in the hope of still adding a
few patches of my own.  Figure I better put them into the next one.