Re: [Qemu-devel] [PATCH 3/3] qobject: replace qobject_incref/QINCREF qobject_decref/QDECREF

[Eric Blake]

On 03/21/2018 09:51 AM, Marc-André Lureau wrote:
> On Wed, Mar 21, 2018 at 3:28 PM, Eric Blake <address@hidden> wrote:
> > On 03/21/2018 08:40 AM, Marc-André Lureau wrote:
> > > Now that we can safely call QOBJECT() on QObject * and children types,
> > > we can have a single macro to ref/unref the object.
> > >
> > > Change the incref/decref prefix name for the more common ref/unref.
> >
> >
> > I know you did this by sed (for the most part); but it would be worth
> > including the command line you used in the commit message to make it easier
> > to reproduce the same mechanical portion of the patch when rebasing.
>
> git sed QINCREF qobject_ref
> git sed QDECREF qobject_unref
> git sed qobject_incref qobject_ref
> git sed qobject_decref qobject_ref

Did any of these create longer lines that needed reflowing?  (Moot point 
if we bikeshed the ending macro name to be no longer than QDECREF)

> > > +++ b/scripts/coccinelle/qobject.cocci
> > > @@ -3,11 +3,11 @@
> > >    expression Obj, Key, E;
> > >    @@
> > >    (
> > > -- qobject_incref(QOBJECT(E));
> > > -+ QINCREF(E);
> > > +- qobject_ref(QOBJECT(E));
> > > ++ qobject_ref(E);
> > >    |
> > > -- qobject_decref(QOBJECT(E));
> > > -+ QDECREF(E);
> > > +- qobject_unref(QOBJECT(E));
> > > ++ qobject_unref(E);
> >
> >
> > I wonder if these branches should just be removed; they made sense when we
> > had two spellings, but after your mass conversion, the .cocci script is
> > unlikely to ever find regressions on this front.
>
> They would still remove extra cast, wouldn't they?

But the point I'm making is that the old code dealing with type-specific 
names actually happened multiple times (it was easy to overlook the 
macro definition that served as shorthand for decrementing a derived 
type, and thus spelling it out in longhand as a decrement on the 
conversion to the base type - and where because the two spellings were 
type-dependent the compiler flagged you if you tried to use the wrong 
spelling for the type at hand); in new code, where a single macro works 
for both base and derived types, no one is likely to mistakenly cast 
their derived type to the base type just to call the generic macro 
(because your naive attempt at just decrementing will work, regardless 
of the type at hand, so you don't have to throw in casts to shut up the 
compiler).

At the same time, you're right that keeping the .cocci doesn't hurt 
(even if it is unlikely to find future offenders).

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