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

Avi Kivity
2010-11-29T09:30:16Z

On 11/28/2010 04:40 PM, Michael S. Tsirkin wrote:
> On Sun, Nov 28, 2010 at 03:14:17PM +0200, Avi Kivity wrote:
> > On 11/28/2010 01:44 PM, Michael S. Tsirkin wrote:
> > >On Sun, Nov 28, 2010 at 11:54:26AM +0200, Avi Kivity wrote:
> > >> On 11/28/2010 11:50 AM, Michael S. Tsirkin wrote:
> > >> >> >
> > >> >> >Another problem is that there seem to be two memory allocations and a
> > >> >> >copy here, apparently just to simplify error handling. It might be fine
> > >> >> >for this test but won't scale for when performance matters.
> > >> >>
> > >> >> When it matters, we can fix it. I don't see msr read/write becoming
> > >> >> a hot path.
> > >> >
> > >> >It will be very painful to fix it.
> > >>
> > >> Why?
> > >
> > >Because the API returns a vector.
> >
> > Returning an object does not involve a copy (return value optimization).
>
> Yes, but assigning the value in the code that uses it will, unless again
> you do this in an initializer.

So do that.

> >
> > This code is not reusable. Everywhere you use an fd, you have to
> > repeat this code.
>
> But that's not a lot of code. And you can abstract it away at a higher
> level. For example kvm_init and kvm_cleanup would setup/cleanup
> state in a consistent way.

That's not what we see in C code. The error handling gets everywhere,
obscuring what's actually going on, and usually getting it wrong by leaking.

> My experience tells me C++ code has much more boilerplate code that you
> are forced to repeat over and over. This is especially true for unix
> system programming: by the time you are done wrapping all of unix you
> have created more LOC than you are ever likely to save.

You don't need to wrap everything. Sometimes you can get away with if
(ret == -1) throw. But things like file descriptors are essential.

> >
> > class kvm::fd is reusable, if you embed it in another object you
> > don't have to worry about errors any more (as long as the object's
> > methods are exception safe).
>
> To get exception safe code, you have to constantly worry about errors.
> And it's easier to spot an unhandled return code than exception-unsafe
> code: gcc actually has __attribute__((warn_unused_result)) which
> might help catch common errors. No such tool to catch
> exception-unsafe code AFAIK.

We see exactly how easy it is by the constant stream of patches that fix
error paths into Linux. Most user space programs don't even care about
errors because they're so difficult and annoying to get right.

> > Yes, all the correctness is more or less pointless here. Like I
> > said, this is an experiment to see what things look like. I guess
> > each side will it as proving its claims.
>
> This is exactly what seems to be happening.
> I did my best to try and be objective and point out real issues,
> but you probably guessed which side I am on already :).

The lack of "the C++ compiler eats babies" and "line noise" comments is
appreciated.

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