| Message ID | 20170609152017.7286-2-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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 >
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.
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
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.
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); |
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(+)