Re: [Xen-devel] [RFC PATCH V3 13/22] Add evtchn_abi_str

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Feb 27, 2013 at 04:51:45PM +0000, Jan Beulich wrote:
> >>> On 27.02.13 at 15:34, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> > --- a/xen/common/event_channel.c
> > +++ b/xen/common/event_channel.c
> > @@ -36,6 +36,20 @@
> >  uint32_t extended_event_channel = (EVTCHN_EXTENDED_NONE |
> >                                     EVTCHN_EXTENDED_L3);
> >  
> > +static inline const char * evtchn_abi_str(unsigned int abi)
> > +{
> > +    switch ( abi )
> > +    {
> > +    case EVTCHN_EXTENDED_NONE:
> > +        return "2-level";
> > +    case EVTCHN_EXTENDED_L3:
> > +        return "3-level";
> > +    default:
> > +        BUG();
> > +    }
> > +    return ""; /* make compiler happy */
> > +}
> > +
> 
> This is the sort of change that looks completely bogus - even the
> next few patches don't seem to make use of this. Why can't this
> be added when the first user of it appears? It surely won't make
> reviewing that patch more difficult...
> 

Do you mean the implementation is bogus or the way I break my patches?

> And that's a general problem (for me at least) with how you break
> up patches: Having looked ahead at 18 and 19, the latter undoes
> quite a significant portion of what the former did. Both being far
> from huge and unreviewable, why don't you fold them so things
> actually make sense to the reader?
> 

Sure, this can be done.


Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel