| Message ID | 20200329045512.GA28203@simran-Inspiron-5558 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] xen/x86: Remove unnecessary cast on void pointer | expand |
On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote: > Assignment to a typed pointer is sufficient in C. > No cast is needed. > > Also, changed some u64/u32 to uint64_t/uint32_t. > > Signed-off-by: Simran Singhal <singhalsimran0@gmail.com> > --- > Changes in v2: > - Took the chance to change some uintX to uintX_t. > > xen/arch/x86/acpi/cpufreq/powernow.c | 2 +- > xen/arch/x86/cpu/vpmu.c | 2 +- > xen/arch/x86/hpet.c | 2 +- > xen/arch/x86/hvm/save.c | 2 +- > xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ > 5 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c > index 3cf9c6cd05..f620bebc7e 100644 > --- a/xen/arch/x86/acpi/cpufreq/powernow.c > +++ b/xen/arch/x86/acpi/cpufreq/powernow.c > @@ -58,7 +58,7 @@ static void transition_pstate(void *pstate) > > static void update_cpb(void *data) > { > - struct cpufreq_policy *policy = (struct cpufreq_policy *)data; > + struct cpufreq_policy *policy = data; > > if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) { > uint64_t msr_content; > diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c > index e50d478d23..1ed39ef03f 100644 > --- a/xen/arch/x86/cpu/vpmu.c > +++ b/xen/arch/x86/cpu/vpmu.c > @@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs) > > static void vpmu_save_force(void *arg) > { > - struct vcpu *v = (struct vcpu *)arg; > + struct vcpu *v = arg; > struct vpmu_struct *vpmu = vcpu_vpmu(v); > > if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) ) > diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c > index 86929b9ba1..c46e7cf4ee 100644 > --- a/xen/arch/x86/hpet.c > +++ b/xen/arch/x86/hpet.c > @@ -215,7 +215,7 @@ again: > static void hpet_interrupt_handler(int irq, void *data, > struct cpu_user_regs *regs) > { > - struct hpet_event_channel *ch = (struct hpet_event_channel *)data; > + struct hpet_event_channel *ch = data; > > this_cpu(irq_count)--; > > diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c > index 0fc59d3487..a2c56fbc1e 100644 > --- a/xen/arch/x86/hvm/save.c > +++ b/xen/arch/x86/hvm/save.c > @@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest, > memcpy(dest, &h->data[h->cur], d->length); > > if ( d->length < dest_len ) > - memset((char *)dest + d->length, 0, dest_len - d->length); > + memset(dest + d->length, 0, dest_len - d->length); I believe you shouldn't drop the cast here either because dest is of type void*. Although the calculation in the end is the same (void* considered of size 1), I would still keep the cast such that the semantics is correct. Wei.
On Sun, Mar 29, 2020 at 02:36:51PM +0100, Wei Liu wrote: > On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote: > > Assignment to a typed pointer is sufficient in C. > > No cast is needed. > > > > Also, changed some u64/u32 to uint64_t/uint32_t. > > > > Signed-off-by: Simran Singhal <singhalsimran0@gmail.com> > > --- > > Changes in v2: > > - Took the chance to change some uintX to uintX_t. > > > > xen/arch/x86/acpi/cpufreq/powernow.c | 2 +- > > xen/arch/x86/cpu/vpmu.c | 2 +- > > xen/arch/x86/hpet.c | 2 +- > > xen/arch/x86/hvm/save.c | 2 +- > > xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ > > 5 files changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c > > index 3cf9c6cd05..f620bebc7e 100644 > > --- a/xen/arch/x86/acpi/cpufreq/powernow.c > > +++ b/xen/arch/x86/acpi/cpufreq/powernow.c > > @@ -58,7 +58,7 @@ static void transition_pstate(void *pstate) > > > > static void update_cpb(void *data) > > { > > - struct cpufreq_policy *policy = (struct cpufreq_policy *)data; > > + struct cpufreq_policy *policy = data; > > > > if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) { > > uint64_t msr_content; > > diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c > > index e50d478d23..1ed39ef03f 100644 > > --- a/xen/arch/x86/cpu/vpmu.c > > +++ b/xen/arch/x86/cpu/vpmu.c > > @@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs) > > > > static void vpmu_save_force(void *arg) > > { > > - struct vcpu *v = (struct vcpu *)arg; > > + struct vcpu *v = arg; > > struct vpmu_struct *vpmu = vcpu_vpmu(v); > > > > if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) ) > > diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c > > index 86929b9ba1..c46e7cf4ee 100644 > > --- a/xen/arch/x86/hpet.c > > +++ b/xen/arch/x86/hpet.c > > @@ -215,7 +215,7 @@ again: > > static void hpet_interrupt_handler(int irq, void *data, > > struct cpu_user_regs *regs) > > { > > - struct hpet_event_channel *ch = (struct hpet_event_channel *)data; > > + struct hpet_event_channel *ch = data; > > > > this_cpu(irq_count)--; > > > > diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c > > index 0fc59d3487..a2c56fbc1e 100644 > > --- a/xen/arch/x86/hvm/save.c > > +++ b/xen/arch/x86/hvm/save.c > > @@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest, > > memcpy(dest, &h->data[h->cur], d->length); > > > > if ( d->length < dest_len ) > > - memset((char *)dest + d->length, 0, dest_len - d->length); > > + memset(dest + d->length, 0, dest_len - d->length); > > I believe you shouldn't drop the cast here either because dest is of > type void*. > > Although the calculation in the end is the same (void* considered of > size 1), I would still keep the cast such that the semantics is correct. IMO dropping the case here is fine, as dest is of type void * the calculation is correct and the cast just obfuscates it. I usually cast things to void * instead of char * or uint8_t * in order to do pointer additions with size 1. Thanks, Roger.
On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote: > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index f049920196..2edb103205 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index) > return offset; > } > > -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding) > +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding) > { > union vmcs_encoding enc; > - u64 *content = (u64 *) vvmcs; > + uint64_t *content = vvmcs; > int offset; > - u64 res; > + uint64_t res; > > enc.word = vmcs_encoding; > offset = vvmcs_offset(enc.width, enc.type, enc.index); > @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding, > return virtual_vmcs_vmread_safe(v, encoding, val); > } > > -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val) > +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val) > { > union vmcs_encoding enc; > - u64 *content = (u64 *) vvmcs; > + uint64_t *content = vvmcs; > int offset; > - u64 res; > + uint64_t res; Thanks for doing the switch of content to type uint64_t. You should however not change the type of res to uint64_t also IMO, as you are not touching that line anyway. With that fixed: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks!
On Mon, Mar 30, 2020 at 3:41 PM Roger Pau Monné <roger.pau@citrix.com> wrote: > On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote: > > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > > index f049920196..2edb103205 100644 > > --- a/xen/arch/x86/hvm/vmx/vvmx.c > > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > > @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 > index) > > return offset; > > } > > > > -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding) > > +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding) > > { > > union vmcs_encoding enc; > > - u64 *content = (u64 *) vvmcs; > > + uint64_t *content = vvmcs; > > int offset; > > - u64 res; > > + uint64_t res; > > > > enc.word = vmcs_encoding; > > offset = vvmcs_offset(enc.width, enc.type, enc.index); > > @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const > struct vcpu *v, u32 encoding, > > return virtual_vmcs_vmread_safe(v, encoding, val); > > } > > > > -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val) > > +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t > val) > > { > > union vmcs_encoding enc; > > - u64 *content = (u64 *) vvmcs; > > + uint64_t *content = vvmcs; > > int offset; > > - u64 res; > > + uint64_t res; > > Thanks for doing the switch of content to type uint64_t. You should > however not change the type of res to uint64_t also IMO, as you are > not touching that line anyway. > Thanks for your feedback Roger and Wei. My bad, I thought I need to change all the unintX to uintX_t in the function set_vvmcs_virtual(). Sorry for the inconvenience. > > With that fixed: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > Thanks! >
On 30.03.2020 12:05, Roger Pau Monné wrote: > On Sun, Mar 29, 2020 at 02:36:51PM +0100, Wei Liu wrote: >> On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote: >>> Assignment to a typed pointer is sufficient in C. >>> No cast is needed. >>> >>> Also, changed some u64/u32 to uint64_t/uint32_t. >>> >>> Signed-off-by: Simran Singhal <singhalsimran0@gmail.com> >>> --- >>> Changes in v2: >>> - Took the chance to change some uintX to uintX_t. >>> >>> xen/arch/x86/acpi/cpufreq/powernow.c | 2 +- >>> xen/arch/x86/cpu/vpmu.c | 2 +- >>> xen/arch/x86/hpet.c | 2 +- >>> xen/arch/x86/hvm/save.c | 2 +- >>> xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ >>> 5 files changed, 10 insertions(+), 10 deletions(-) >>> >>> diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c >>> index 3cf9c6cd05..f620bebc7e 100644 >>> --- a/xen/arch/x86/acpi/cpufreq/powernow.c >>> +++ b/xen/arch/x86/acpi/cpufreq/powernow.c >>> @@ -58,7 +58,7 @@ static void transition_pstate(void *pstate) >>> >>> static void update_cpb(void *data) >>> { >>> - struct cpufreq_policy *policy = (struct cpufreq_policy *)data; >>> + struct cpufreq_policy *policy = data; >>> >>> if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) { >>> uint64_t msr_content; >>> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c >>> index e50d478d23..1ed39ef03f 100644 >>> --- a/xen/arch/x86/cpu/vpmu.c >>> +++ b/xen/arch/x86/cpu/vpmu.c >>> @@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs) >>> >>> static void vpmu_save_force(void *arg) >>> { >>> - struct vcpu *v = (struct vcpu *)arg; >>> + struct vcpu *v = arg; >>> struct vpmu_struct *vpmu = vcpu_vpmu(v); >>> >>> if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) ) >>> diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c >>> index 86929b9ba1..c46e7cf4ee 100644 >>> --- a/xen/arch/x86/hpet.c >>> +++ b/xen/arch/x86/hpet.c >>> @@ -215,7 +215,7 @@ again: >>> static void hpet_interrupt_handler(int irq, void *data, >>> struct cpu_user_regs *regs) >>> { >>> - struct hpet_event_channel *ch = (struct hpet_event_channel *)data; >>> + struct hpet_event_channel *ch = data; >>> >>> this_cpu(irq_count)--; >>> >>> diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c >>> index 0fc59d3487..a2c56fbc1e 100644 >>> --- a/xen/arch/x86/hvm/save.c >>> +++ b/xen/arch/x86/hvm/save.c >>> @@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest, >>> memcpy(dest, &h->data[h->cur], d->length); >>> >>> if ( d->length < dest_len ) >>> - memset((char *)dest + d->length, 0, dest_len - d->length); >>> + memset(dest + d->length, 0, dest_len - d->length); >> >> I believe you shouldn't drop the cast here either because dest is of >> type void*. >> >> Although the calculation in the end is the same (void* considered of >> size 1), I would still keep the cast such that the semantics is correct. > > IMO dropping the case here is fine, as dest is of type void * the > calculation is correct and the cast just obfuscates it. +1 Jan
On 30.03.2020 12:11, Roger Pau Monné wrote: > On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote: >> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c >> index f049920196..2edb103205 100644 >> --- a/xen/arch/x86/hvm/vmx/vvmx.c >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c >> @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index) >> return offset; >> } >> >> -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding) >> +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding) >> { >> union vmcs_encoding enc; >> - u64 *content = (u64 *) vvmcs; >> + uint64_t *content = vvmcs; >> int offset; >> - u64 res; >> + uint64_t res; >> >> enc.word = vmcs_encoding; >> offset = vvmcs_offset(enc.width, enc.type, enc.index); >> @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding, >> return virtual_vmcs_vmread_safe(v, encoding, val); >> } >> >> -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val) >> +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val) >> { >> union vmcs_encoding enc; >> - u64 *content = (u64 *) vvmcs; >> + uint64_t *content = vvmcs; >> int offset; >> - u64 res; >> + uint64_t res; > > Thanks for doing the switch of content to type uint64_t. You should > however not change the type of res to uint64_t also IMO, as you are > not touching that line anyway. I actually wouldn't mind the patch being left as is? > With that fixed: > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> Roger - please clarify if your R-b is also fine without the requested adjustment. Jan
On Mon, Mar 30, 2020 at 01:09:20PM +0200, Jan Beulich wrote: > On 30.03.2020 12:11, Roger Pau Monné wrote: > > On Sun, Mar 29, 2020 at 10:25:12AM +0530, Simran Singhal wrote: > >> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > >> index f049920196..2edb103205 100644 > >> --- a/xen/arch/x86/hvm/vmx/vvmx.c > >> +++ b/xen/arch/x86/hvm/vmx/vvmx.c > >> @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index) > >> return offset; > >> } > >> > >> -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding) > >> +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding) > >> { > >> union vmcs_encoding enc; > >> - u64 *content = (u64 *) vvmcs; > >> + uint64_t *content = vvmcs; > >> int offset; > >> - u64 res; > >> + uint64_t res; > >> > >> enc.word = vmcs_encoding; > >> offset = vvmcs_offset(enc.width, enc.type, enc.index); > >> @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding, > >> return virtual_vmcs_vmread_safe(v, encoding, val); > >> } > >> > >> -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val) > >> +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val) > >> { > >> union vmcs_encoding enc; > >> - u64 *content = (u64 *) vvmcs; > >> + uint64_t *content = vvmcs; > >> int offset; > >> - u64 res; > >> + uint64_t res; > > > > Thanks for doing the switch of content to type uint64_t. You should > > however not change the type of res to uint64_t also IMO, as you are > > not touching that line anyway. > > I actually wouldn't mind the patch being left as is? > > > With that fixed: > > > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > Acked-by: Jan Beulich <jbeulich@suse.com> > > Roger - please clarify if your R-b is also fine without the requested > adjustment. Yes, TBH I was borderline on requesting the change, as the type change is correct. Roger.
diff --git a/xen/arch/x86/acpi/cpufreq/powernow.c b/xen/arch/x86/acpi/cpufreq/powernow.c index 3cf9c6cd05..f620bebc7e 100644 --- a/xen/arch/x86/acpi/cpufreq/powernow.c +++ b/xen/arch/x86/acpi/cpufreq/powernow.c @@ -58,7 +58,7 @@ static void transition_pstate(void *pstate) static void update_cpb(void *data) { - struct cpufreq_policy *policy = (struct cpufreq_policy *)data; + struct cpufreq_policy *policy = data; if (policy->turbo != CPUFREQ_TURBO_UNSUPPORTED) { uint64_t msr_content; diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c index e50d478d23..1ed39ef03f 100644 --- a/xen/arch/x86/cpu/vpmu.c +++ b/xen/arch/x86/cpu/vpmu.c @@ -337,7 +337,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs) static void vpmu_save_force(void *arg) { - struct vcpu *v = (struct vcpu *)arg; + struct vcpu *v = arg; struct vpmu_struct *vpmu = vcpu_vpmu(v); if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) ) diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c index 86929b9ba1..c46e7cf4ee 100644 --- a/xen/arch/x86/hpet.c +++ b/xen/arch/x86/hpet.c @@ -215,7 +215,7 @@ again: static void hpet_interrupt_handler(int irq, void *data, struct cpu_user_regs *regs) { - struct hpet_event_channel *ch = (struct hpet_event_channel *)data; + struct hpet_event_channel *ch = data; this_cpu(irq_count)--; diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c index 0fc59d3487..a2c56fbc1e 100644 --- a/xen/arch/x86/hvm/save.c +++ b/xen/arch/x86/hvm/save.c @@ -417,7 +417,7 @@ void _hvm_read_entry(struct hvm_domain_context *h, void *dest, memcpy(dest, &h->data[h->cur], d->length); if ( d->length < dest_len ) - memset((char *)dest + d->length, 0, dest_len - d->length); + memset(dest + d->length, 0, dest_len - d->length); h->cur += d->length; } diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c index f049920196..2edb103205 100644 --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -253,12 +253,12 @@ static int vvmcs_offset(u32 width, u32 type, u32 index) return offset; } -u64 get_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding) +uint64_t get_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding) { union vmcs_encoding enc; - u64 *content = (u64 *) vvmcs; + uint64_t *content = vvmcs; int offset; - u64 res; + uint64_t res; enc.word = vmcs_encoding; offset = vvmcs_offset(enc.width, enc.type, enc.index); @@ -307,12 +307,12 @@ enum vmx_insn_errno get_vvmcs_real_safe(const struct vcpu *v, u32 encoding, return virtual_vmcs_vmread_safe(v, encoding, val); } -void set_vvmcs_virtual(void *vvmcs, u32 vmcs_encoding, u64 val) +void set_vvmcs_virtual(void *vvmcs, uint32_t vmcs_encoding, uint64_t val) { union vmcs_encoding enc; - u64 *content = (u64 *) vvmcs; + uint64_t *content = vvmcs; int offset; - u64 res; + uint64_t res; enc.word = vmcs_encoding; offset = vvmcs_offset(enc.width, enc.type, enc.index);
Assignment to a typed pointer is sufficient in C. No cast is needed. Also, changed some u64/u32 to uint64_t/uint32_t. Signed-off-by: Simran Singhal <singhalsimran0@gmail.com> --- Changes in v2: - Took the chance to change some uintX to uintX_t. xen/arch/x86/acpi/cpufreq/powernow.c | 2 +- xen/arch/x86/cpu/vpmu.c | 2 +- xen/arch/x86/hpet.c | 2 +- xen/arch/x86/hvm/save.c | 2 +- xen/arch/x86/hvm/vmx/vvmx.c | 12 ++++++------ 5 files changed, 10 insertions(+), 10 deletions(-)