Re: [Xen-devel] [PATCH for-next v3 19/22] x86: clean up pv/traps.c

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.05.17 at 19:10, <wei.liu2@xxxxxxxxxx> wrote:
> Fix coding style issues. No functional change.

Same general comment as on the earlier cleanup patch.

> @@ -318,17 +325,19 @@ int send_guest_trap(struct domain *d, uint16_t vcpuid, 
> unsigned int trap_nr)
>              return -EBUSY;
>  
>          /* We are called by the machine check (exception or polling) handlers
> -         * on the physical CPU that reported a machine check error. */
> +         * on the physical CPU that reported a machine check error.
> +         */

Please correct the comment as a whole, not just one of its ends.

> @@ -448,27 +458,31 @@ static long register_guest_callback(struct 
> callback_register *reg)
>      switch ( reg->type )
>      {
>      case CALLBACKTYPE_event:
> -        v->arch.pv_vcpu.event_callback_eip    = reg->address;
> +        v->arch.pv_vcpu.event_callback_eip = reg->address;
>          break;
>  
>      case CALLBACKTYPE_failsafe:
>          v->arch.pv_vcpu.failsafe_callback_eip = reg->address;
> +
>          if ( reg->flags & CALLBACKF_mask_events )
>              set_bit(_VGCF_failsafe_disables_events,
>                      &v->arch.vgc_flags);
>          else
>              clear_bit(_VGCF_failsafe_disables_events,
>                        &v->arch.vgc_flags);
> +
>          break;
>  
>      case CALLBACKTYPE_syscall:
>          v->arch.pv_vcpu.syscall_callback_eip  = reg->address;
> +
>          if ( reg->flags & CALLBACKF_mask_events )
>              set_bit(_VGCF_syscall_disables_events,
>                      &v->arch.vgc_flags);
>          else
>              clear_bit(_VGCF_syscall_disables_events,
>                        &v->arch.vgc_flags);
> +
>          break;

Some of the earlier additions of blank lines were already
questionable imo, but especially the ones you add here before the
break statements go to far afaic.

> @@ -674,13 +688,16 @@ void compat_show_guest_stack(struct vcpu *v, const 
> struct cpu_user_regs *regs,
>          printk(" %08x", addr);
>          stack++;
>      }
> +
>      if ( mask == PAGE_SIZE )
>      {
>          BUILD_BUG_ON(PAGE_SIZE == STACK_SIZE);
>          unmap_domain_page(stack);
>      }
> +
>      if ( i == 0 )
>          printk("Stack empty.");
> +
>      printk("\n");
>  }

Same for at least the last one here.

Jan


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