Re: [Xen-devel] [PATCH v3 03/15] xen: Relocate mem_event_op domctl and access_op memop into common.

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Tue, Sep 2, 2014 at 8:30 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 01.09.14 at 18:25, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> On Mon, Sep 1, 2014 at 5:46 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
>> >>> On 01.09.14 at 16:21, <tklengyel@xxxxxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/x86_64/compat/mm.c
>> > +++ b/xen/arch/x86/x86_64/compat/mm.c
>> > @@ -198,10 +198,6 @@ int compat_arch_memory_op(unsigned long cmd,
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>> >          break;
>> >      }
>> >
>> > -    case XENMEM_access_op:
>> > -        rc = mem_access_memop(cmd, guest_handle_cast(arg,
>> xen_mem_access_op_t));
>> > -        break;
>> > -
>>
>> I don't think you can simply drop this.
>>
>
> OK.
>
>
>>
>> > @@ -967,6 +968,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
>> u_domctl)
>> >      }
>> >      break;
>> >
>> > +    case XEN_DOMCTL_mem_event_op:
>> > +    {
>> > +        ret = mem_event_domctl(d, &op->u.mem_event_op,
>> > +                              guest_handle_cast(u_domctl, void));
>> > +        copyback = 1;
>> > +    }
>> > +    break;
>>
>> Please drop the unnecessary braces.
>

> All other cases have braces around them here, even when not required, so
> this just follows the established style.

This is so because everyone uses that same argument, blindly
putting braces in place even when they're not needed. As pointed
out recently to someone else (also changing a domctl, istr it was
Aravindh), this breaks indentation of brace-enclosed blocks, so
should be used only when indeed needed.

Jan

Makes sense, I'll fix the style in the next iteration.

Tamas

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