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 <math...@castello.eng.br> 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 <math...@castello.eng.br>
---
  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