> -----Original Message----- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 16 December 2016 10:25 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; xen-devel <xen- > de...@lists.xenproject.org> > Subject: RE: [PATCH] x86/HVM: handle_{mmio*,pio}() return value > adjustments > > >>> On 16.12.16 at 10:52, <paul.durr...@citrix.com> wrote: > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: 16 December 2016 09:31 > >> @@ -100,22 +100,21 @@ int handle_mmio(void) > >> { > >> case X86EMUL_UNHANDLEABLE: > >> hvm_dump_emulation_state(XENLOG_G_WARNING "MMIO", > &ctxt); > >> - return 0; > >> + return false; > >> + > >> case X86EMUL_EXCEPTION: > >> if ( ctxt.ctxt.event_pending ) > >> hvm_inject_event(&ctxt.ctxt.event); > >> break; > >> - default: > >> - break; > > > > Should there not be some sort of default case, even if it's simply to assert > > that it's not reachable? > > A default case doing nothing when the switch expression is not of > an enum type is pointless. And it _is_ reachable (namely for > X86EMUL_OKAY).
Then my preference would still be an explicit case for X86EMUL_OKAY and an unreachable default, but if you don't think it's worth it then the code is ok as-is. > > >> --- a/xen/arch/x86/hvm/ioreq.c > >> +++ b/xen/arch/x86/hvm/ioreq.c > >> @@ -156,13 +156,14 @@ bool_t handle_hvm_io_completion(struct v > > > > Do you not want to change this to from bool_t to bool while you're at it? > > Well, while I first was inclined to do so, I then didn't want to do > too many things at once (iirc there would be a few more which then > would want updating at the same time). > Fair enough. Reviewed-by: Paul Durrant <paul.durr...@citrix.com> > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
