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

Michael S. Tsirkin
2010-11-28T16:58:20Z

On Sun, Nov 28, 2010 at 04:34:39PM +0200, Avi Kivity wrote:
> On 11/28/2010 03:57 PM, Michael S. Tsirkin wrote:
> >On Sun, Nov 28, 2010 at 03:02:18PM +0200, Avi Kivity wrote:
> >> On 11/28/2010 01:59 PM, Michael S. Tsirkin wrote:
> >> >>
> >> >>
> >> >> FWIW, I still disagree with C++ and believe this code to be hardly readable.
> >> >
> >> >A major issue is existing tools.
> >> >
> >> >Using C++ would prevent us from using sparce for static code checking.
> >>
> >> C++ static checking is way better than anything sparse offers.
> >
> >This seems a second system effect at work.
> >
> >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, void, builtin
types, sizeof, arrays, NULL comparizon, inheritance, cast,
memory management.

Examples to ponder: what is the appropriate value of T for void *?
What if you want a shared/auto ptr to manage this memory?

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.

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

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

> C : struct T_user_ptr { unsigned long addr } foo; + lots of accessors.

Some kind of macro can be closer to user_ptr above.

> >> >We should be adding more annotations instead of throwing existing ones
> >> >out. ctags is also broken with C++ which will make it much harder
> >> >for me to browse the codebase.
> >>
> >> C++ does want a good IDE.
> >
> >For some definitions of good :).
> >Let's start a vi versus emacs flamewar?
>
> Like we haven't enough...
>
> >> >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.

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

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

> >> >I also hoped we'll be able to adopt checkpatch at some point for coding
> >> >style enforcement, C++ syntax is just too complex for a perl script to
> >> >be of any use.
> >>
> >> Not much of a loss IMO.
> >
> >This is just an example of how coding style is harder to
> >specify and enforce. There are just much more ways to do
> >any given thing. Uniform style is much harder to achieve.
>
> The language does more, so it's more complicated.

We seem to have trouble enough making the style uniform with C.

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