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

Avi Kivity
2010-11-29T09:22:51Z

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