[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> 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...

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?

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.