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

[Avi Kivity <avi@redhat.com>]

On 11/24/2010 08:23 PM, Gleb Natapov wrote:
> On Wed, Nov 24, 2010 at 07:50:06PM +0200, Avi Kivity wrote:
> >  On 11/24/2010 07:41 PM, Gleb Natapov wrote:
> >  >On Wed, Nov 24, 2010 at 07:39:39PM +0200, Avi Kivity wrote:
> >  >>   On 11/24/2010 07:33 PM, Gleb Natapov wrote:
> >  >>   >>
> >  >>   >>    But I don't see the point.  If you look at my repository, there's
> >  >>   >The point is that C++ is ugly language. The short code Avi sent remind
> >  >>   >me perl (aka line noise). It is almost impossible to parse it into
> >  >>   >what code it actually does. Most symbols are there for C++ syntax not
> >  >>   >functionality.
> >  >>
> >  >>   No.  They're there for error handling.  A C++ wrapper, doesn't add
> >  >>   any functionality, so you can say that all of the lines of codes do
> >  >>   nothing and are just syntax.  But they do allow you to pair init and
> >  >>   uninit (in the constructor and destructor).  When you use the
> >  >>   wrapper (as opposed to the bare C interface) you get the value by
> >  >>   not having to code long error handling sequences (with a high
> >  >>   probability of getting them wrong and never finding out in testing).
> >  >>
> >  >So how errors are handled there? By throwing exceptions? Sorry this is
> >  >not error "handling".
> >
> >  It's not in this code, but in it's callers.
> >
> >    kvm::system sys;
> >    kvm::vm vm(sys);
> >    kvm::vcpu vcpu(vm, 0);
> >    mutex::guard guard(some_mutex);
> >    some_other_object blah;
> >    return do_something();
> >
> >  If an exception happens in any of these places, whatever's already
> >  constructed is rolled back and destroyed.  Contrast with
> >
> >    struct kvm_system sys;
> >    struct kvm_vm vm;
> >    struct kvm_vcpu vcpu;
> >    struct some_other_object object;
> >    int r;
> >
> >    r = kvm_system_init(&sys);
> >    if (r<  0)
> >        goto out_1;
> >    r = kvm_vm_init(&vm,&sys);
> >    if (r<  0)
> >        goto out_2;
> >    r = kvm_vcpu_init(&vcpu,&vm, 0);
> >    if (r<  0)
> >        goto out_3;
> >    mutex_aquire(&some_mutex);
> >    r = some_other_object_init(&object);
> >    if (r<  0)
> >        goto out_4;
> >    r = do_something();
> >    some_other_object_destroy(&object);
> >  out_4:
> >    mutex_release(&mutex);
> >  out_3:
> >    kvm_vcpu_destroy(&vcpu);
> >  out_2:
> >    kvm_vm_destroy(&vm);
> >  out_1:
> >    kvm_system_destroy(&sys);
> >    return r;
> >
> >  Which, in your opinion, is more readable?
> >
> It is not realistic example since you create objects on the stack which
> in real code will be on a heap.

Those are still the same issues and are solved in the same way.  Linux 
is riddled with these patterns and there are always patches in flight to 
fix error handling paths.

In fact with heap based objects C++ is even more compelling since you 
can do automatic reference counting.

>   And in your short C++ example
> mutex::guard guard(some_mutex); part is ugly as hell. "Protect code from
> here to end of the scope"?! Later somebody will add code at the end of
> the function that will run under mutex needlessly. Proper
> language allow you to protect scope with mutex like that:
>   with(some_mutex) {
> 	some_other_object blah;
>   }

I agree.  You can always introduce a new block scope though.

I prefer the ugliness to having to roll back the stack manually.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.