On Sun, Nov 28, 2010 at 11:31:09AM +0200, Avi Kivity wrote: > On 11/28/2010 10:58 AM, Michael S. Tsirkin wrote: > >On Sat, Nov 27, 2010 at 11:12:58AM +0200, Avi Kivity wrote: > >> On 11/26/2010 12:16 PM, Michael S. Tsirkin wrote: > >> >On Wed, Nov 24, 2010 at 12:52:11PM +0200, Avi Kivity wrote: > >> >> Introduce exception-safe objects for calling system, vm, and vcpu ioctls. > >> >> > >> >> Signed-off-by: Avi Kivity<avi@xxxxxxxxxx> > >> > > >> >ioctlp calls below ignore possible errors. > >> >Somre more comments below. > >> > > >> > >> Can you elaborate? The simply propagate the exception. > > > >I was confused by this: > > > >+ long ioctlp(unsigned nr, void *arg) { > >+ return ioctl(nr, reinterpret_cast<long>(arg)); > >+ } > > > > > >ioctl here is not the C ioctl function. It's the local method that > >throws exceptions on errors. This will likely confuse others > >as well. > > Could do this->ioctl(), though I don't much like it. > >> >> + > >> >> +std::vector<kvm_msr_entry> vcpu::msrs(std::vector<uint32_t> indices) > >> >> +{ > >> >> + std::auto_ptr<kvm_msrs> msrs(alloc_msr_list(indices.size())); > >> > > >> >This looks wrong. auto_ptr frees memory with delete, > >> >alloc_msr_list allocates it with malloc. > >> > >> Anthony pointed this out as well. > > > >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. > >> Fixed by replacing > >> alloc_msr_list() by an object. > > > >It seems that any action which has side effects which needs to be > >undone on error, we will have to have a new class with constructor doing > >the work. This will likely create much more lines of code > >than a simple goto end strategy. > > It creates correctness. The equivalent in qemu is to create a > constant size array on the stack, because people can't be bothered > with error checking. > > The lines of code pay back as soon as there is some reuse (2x in this case). > > >One also wonders how well will the compiler be able to optimize > >such complex code, and by how much will compile times go up. > > > >Any input on that? > > The compiler should optimize it away completely. Should as opposed to does. Want me to try a simple test? > There's been a lot > of work in gcc on that. > > About compile times, I don't care much. I do. You will too when we have codebase that can be built as fast as we commit things, so buildbot breaks. This is common in C++ based projects. > -- > error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html