Re: [PATCH 4/6] drivers: hv: vmbus: Fix checkpatch SPLIT_STRING

[Matheus Castello]

Hi Michael,

Em 11/15/2020 7:25 PM, Michael Kelley escreveu:
> From: Matheus Castello <matheus@xxxxxxxxxxxxxxx> Sent: Sunday, November 15, 2020 11:58 AM
> > Checkpatch emits WARNING: quoted string split across lines.
> > To keep the code clean and with the 80 column length indentation the
> > check and registration code for kmsg_dump_register has been transferred
> > to a new function hv_kmsg_dump_register.
> >
> > Signed-off-by: Matheus Castello <matheus@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/hv/vmbus_drv.c | 34 +++++++++++++++++++---------------
> >   1 file changed, 19 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 61d28c743263..09d8236a51cf 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -1387,6 +1387,23 @@ static struct kmsg_dumper hv_kmsg_dumper = {
> >   	.dump = hv_kmsg_dump,
> >   };
> >
> > +static void hv_kmsg_dump_register(void)
> > +{
> > +	int ret;
> > +
> > +	hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> > +	if (hv_panic_page) {
> > +		ret = kmsg_dump_register(&hv_kmsg_dumper);
> > +		if (ret) {
> > +			pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
> > +			hv_free_hyperv_page((unsigned long)hv_panic_page);
> > +			hv_panic_page = NULL;
> > +		}
> > +	} else {
> > +		pr_err("Hyper-V: panic message page memory allocation failed");
> > +	}
> > +}
> > +
>
> The above would be marginally better if organized as follows so that the
> main execution path isn't in an "if" clause.  Also reduces indentation.
>
> 	hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> 	if (!hv_panic_page) {
> 		pr_err("Hyper-V: panic message page memory allocation failed");
> 		return;
> 	}
> 	ret = kmsg_dump_register(&hv_kmsg_dumper);
> 	if (ret) {
> 		pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
> 		hv_free_hyperv_page((unsigned long)hv_panic_page);
> 		hv_panic_page = NULL;
> 	}

Thanks for the review, great I will use it on the v2.

> >   static struct ctl_table_header *hv_ctl_table_hdr;
> >
> >   /*
> > @@ -1477,21 +1494,8 @@ static int vmbus_bus_init(void)
> >   		 * capability is supported by the hypervisor.
> >   		 */
> >   		hv_get_crash_ctl(hyperv_crash_ctl);
> > -		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
> > -			hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> > -			if (hv_panic_page) {
> > -				ret = kmsg_dump_register(&hv_kmsg_dumper);
> > -				if (ret) {
> > -					pr_err("Hyper-V: kmsg dump register "
> > -						"error 0x%x\n", ret);
> > -					hv_free_hyperv_page(
> > -					    (unsigned long)hv_panic_page);
> > -					hv_panic_page = NULL;
> > -				}
> > -			} else
> > -				pr_err("Hyper-V: panic message page memory "
> > -					"allocation failed");
> > -		}
> > +		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG)
> > +			hv_kmsg_dump_register();
> >
> >   		register_die_notifier(&hyperv_die_block);
> >   	}
> > --
> > 2.28.0