Re: [PATCH kvm-unit-tests 2/4] Introduce a C++ wrapper for the kvm APIs

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Nov 29, 2010 at 11:22:44AM +0200, Avi Kivity wrote:
> >>  No need for an additional toolchain.
> >
> >It's a feature :) This way  you are not forced to rewrite all code each
> >time you realize you need an extra check, and checks can be added
> >gradually without breaking build.
> 
> You can see that user_ptr<> is not just for the checks, it adds
> functionality (sizeof-less copy_from and copy_to).  That's usually
> the case.  If there's something you must not do because of some
> rule, there's also something you want to do, and those become member
> functions.
> 
> In C++ you could also introduce user_ptr<> gradually, it won't break
> anything.

Yes but in
	void foo(void *p) {
		bar(p);
	}
both foo and bar must be converted.

> >>  >>
> >>  >>   Things like __user are easily done in C++.
> >>  >
> >>  >Some of what sparce does can be done if you create a separate type for
> >>  >all address spaces.  This can be done in C too, and the result won't
> >>  >be like __user at all.
> >>
> >>  That's quite a lot of work.
> >>
> >>   Sparse:  T __user *foo;
> >>   C++: user_ptr<T>  foo;
> >
> >Sparse has some advantages: it makes the contract obvious so you clearly
> >see it's a pointer and know ->, [], + will work, * and<<  will not.
> 
> I don't really see how you can tell this from __user.

I can tell this is a pointer from T *foo :), and I can tell
it has some attribute.

>  You have to
> look up the definition.  For user_ptr<>, the definition is actually
> available.

The definition for __user is also available:
#ifdef __CHECKER__
# define __user                __attribute__((noderef, address_space(1)))
#else
# define __user
#endif

Which is a very transparent way to say: this is just a checker
attribute, it does not affect actual code.

With a template we go 'I have overridden + but compiler should
optimize it back to original'. Note the should :)

> >>  >>    Templates are
> >>  >>   indeed harder to debug, simply because names can become very long.
> >>  >
> >>  >That's not the only problem. A bigger one is when you type tab to
> >>  >complete function name and get a list of options to select from for each
> >>  >of the times a template was instantiated.  Only one of them is relevant
> >>  >in a given scope. No hint is given which.  Further when you step into
> >>  >the template, the source does not give you any hint about the types
> >>  >used.
> >>  >
> >>  >Some of this is true for macros as well of course. Except people know
> >>  >macros are bad and so make them thin wrappers around proper functions.
> >>
> >>  Or they simply avoid it and duplicate the code.  You can't always
> >>  wrap functions with macros.
> >
> >Always is a strong word.  What are the examples of such duplicated code
> >in qemu?  Let's see if they are easy to fix.
> 
> qemu in fact uses macros extensively (glue()), they are hardly readable.

Well, we don't have so many instances left anymore.

We used to have this in pci and I got rid of it pretty easily,
just passing in length at runtime. And it turned out the only reason
for it was because we didn't pass in the transaction length.
That technique (clean up APIs so we don't have to work around
them with macros) leads IMO to better code than just sticking
a tamplate around it.

> Do a 'git grep hash' for examples of duplication.

I see. Don't know enough about tcg to fix unfortunately, but
the logic is different: e.g. sparc opcode table is completely
static, translation cache is dynamic, jump cache has
static size but dynamic content, qdict could do
with linear lookups just as well. So it could
just be different optimization strategies.

> -- 
> error compiling committee.c: too many arguments to function