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

[Avi Kivity <avi@redhat.com>]

On 11/28/2010 06:57 PM, Michael S. Tsirkin wrote:
> >  >
> >  >sparce lets you solve C problems that C++ inherited as is.
> >  >E.g. if you have a pointer you can always dereference it.
> >
> >  It's the other way round.
> >
> >  For example __user cannot be done in C.  It has to be done as an add-on.
> >
> >  In C++ it's simply:
> >
> >    template<class T>
> >    class user_ptr<T>
> >    {
> >    public:
> >        explicit user_ptr(unsigned long addr);
> >        void copy_from(T&  to); // throws EFAULT
> >        void copy_to(const T&  from); // throws EFAULT
> >    private:
> >        unsigned long addr;
> >    };
>
> This does not allow simple uses such as arithmetic,

Add a raw_addr() method that returns addr.

> void,

Fixable.

>   builtin
> types,

should work

>   sizeof,

sizeof(T)?

> arrays,

should work

>   NULL comparizon,

Do we ever compare __user pointers against NULL?  It's  a valid address.

> inheritance, cast,

What do these mean in the context of user pointers?

> memory management.

And this?

> Examples to ponder: what is the appropriate value of T for void *?

Probably a specialization user_ptr<> or a separate class user_void_ptr.  
After all you can't do anthing with such a pointer.

> What if you want a shared/auto ptr to manage this memory?

What does it mean for user pointers?

> Some of these might be fixable, with a lot of code.
> Boost might haver some solutions, I haven't looked.
>
> Meanwhile sparse is already there.

With sparse you have to implement every rule in a separate compiler.  
With C++ you introduce the rules into the code.

> >  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.

> >  >>
> >  >>   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.  You have to look 
up the definition.  For user_ptr<>, the definition is actually available.

> >   C : struct T_user_ptr { unsigned long addr } foo; + lots of accessors.
>
> Some kind of macro can be closer to user_ptr above.

Those macros are called templates, and the compiler can check that they 
are used correctly.


> >
> >  >>   >C++ support in gdb has some limitations
> >  >>   >if you use overloading, exceptions, templates. The example posted here
> >  >>   >uses two of these, so it would be harder to debug.
> >  >>
> >  >>   I haven't seen issues with overloading or exceptions.
> >  >
> >  >Build your test with -g, fire up gdb from command line,
> >  >try to put a breakpoint in the constructor of
> >  >the fd object, maybe you will see what I mean :)
> >  >
> >  (gdb) break 'kvm::fd::fd'
> >  Breakpoint 3 at 0x8049650: file api/kvmxx.cc, line 25.
> >  Breakpoint 4 at 0x8049628: file api/kvmxx.cc, line 31.
> >  Breakpoint 5 at 0x8049080: file api/kvmxx.cc, line 21
>
> But it's hard to figure out that you need the kvm namespace.  Your code
> only has one namespace, but with multiple namespaces, you don't even
> know in which namespace to look up the fd.  With templates you might not
> even know the fd class.

If you like, you can avoid namespaces and prefix everything with kvm_.  
I never found it necessary.

> >  >An example of an issue with overloading is that gdb seems unable to resolve
> >  >them properly depending on the current scope.  So you see a call to
> >  >foo() and want to put a breakpoint there, first problem is just to find
> >  >one which namespace it is in. Once you did the best
> >  >it can do it prompt you to select one of the overloaded options.
> >  >How do you know which one do you want? You don't, so you guess. Sometimes
> >  >gdb will guess, because of a complex set of name resolution rules, and
> >  >sometimes it will this wrongly.
> >  >Which is not what I want to spend mental cycles on when I am debugging a problem.
> >  >
> >  >Functions using exceptions can not be called from the gdb prompt
> >  >(gdb is not smart enough to catch them).
> >  >
> >  >There are more issues.
> >
> >  That's not restricted to gdb.  C has just three scopes: block (may
> >  be nested), file static, and global.  C++ has more.  Stating
> >  everything leads to verbose code and potential conflicts.  Having
> >  more scopes allows tighter code usually
> >
> >  but more head-scratching if
> >  something goes wrong.
> >
> >  In my experience conflicts are very rare.  But it's true that when
> >  they happen they can be surprising.
>
> There is some difference: when you write code, conflicts are rare
> because you let the compiler guess the right call from the scope.  When
> you read code or debug, conflicts are everywhere.  They are rarely
> surprising, they are always time consuming to resolve.

I guess this is subjective.  If you use good tools this shouldn't happen 
though.

> >  >>    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.

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


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