On Tue, 26 Feb 2013 15:20:52 +0100 Stefan Hajnoczi <stefanha@xxxxxxxxx> wrote: > On Tue, Feb 26, 2013 at 12:55:36PM +0200, Michael S. Tsirkin wrote: > > On Mon, Feb 25, 2013 at 04:27:49PM +0100, Cornelia Huck wrote: > > > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c > > > index f0ced1a..8de3cd7 100644 > > > --- a/virt/kvm/eventfd.c > > > +++ b/virt/kvm/eventfd.c > > > @@ -679,11 +679,16 @@ static int > > > kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > > > { > > > int pio = args->flags & KVM_IOEVENTFD_FLAG_PIO; > > > - enum kvm_bus bus_idx = pio ? KVM_PIO_BUS : KVM_MMIO_BUS; > > > + int ccw; > > > + enum kvm_bus bus_idx; > > > struct _ioeventfd *p; > > > struct eventfd_ctx *eventfd; > > > int ret; > > > > > > + ccw = args->flags & KVM_IOEVENTFD_FLAG_VIRTIO_CCW_NOTIFY; > > > + bus_idx = pio ? KVM_PIO_BUS : > > > + ccw ? KVM_VIRTIO_CCW_NOTIFY_BUS : > > > + KVM_MMIO_BUS; > > > > May be better to rewrite using if/else. > > Saw this after sending my comment. I agree with Michael, an if > statement allows you to drop the locals and capture the bus_idx > conversion in a single place (it could even be a static function to save > duplicating the code in both functions that use it). > > Stefan > Introducing a helper function sounds reasonable. -- 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