On Tue, Aug 15, 2023 at 10:41 AM <crash-utility-request@redhat.com> wrote:
Date: Mon, 14 Aug 2023 09:41:12 -0400
From: Rafael Aquini <aquini@redhat.com>
To: crash-utility@redhat.com
Cc: raquini@redhat.com
Subject: [Crash-utility] [PATCH] deduplicate kernel_version open-coded
        parser
Message-ID: <20230814134112.232436-1-aquini@redhat.com>

The code that parses kernel version from OSRELEASE/UTSRELEASE strings
and populates the global kernel table is duplicated across the codebase
for no good reason. This commit consolidates all the duplicated parsing
code into a single method to remove the unnecessary duplicated code.


Thank you for the patch, Rafael.

And this looks good to me, so:  Ack.

Thanks.
Lianbo

 
Signed-off-by: Rafael Aquini <aquini@redhat.com>
---
 arm64.c   | 27 +++----------------
 defs.h    |  2 ++
 kernel.c  | 77 ++++++++++++++++++++++++++-----------------------------
 riscv64.c | 25 ++----------------
 4 files changed, 43 insertions(+), 88 deletions(-)

diff --git a/arm64.c b/arm64.c
index 67b1a22..39d5f04 100644
--- a/arm64.c
+++ b/arm64.c
@@ -834,35 +834,14 @@ static struct kernel_va_range_handler kernel_va_range_handlers[] = {
 static unsigned long arm64_get_kernel_version(void)
 {
        char *string;
-       char buf[BUFSIZE];
-       char *p1, *p2;

        if (THIS_KERNEL_VERSION)
                return THIS_KERNEL_VERSION;

-       string = pc->read_vmcoreinfo("OSRELEASE");
-       if (string) {
-               strcpy(buf, string);
-
-               p1 = p2 = buf;
-               while (*p2 != '.')
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[0] = atoi(p1);
-
-               p1 = ++p2;
-               while (*p2 != '.')
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[1] = atoi(p1);
-
-               p1 = ++p2;
-               while ((*p2 >= '0') && (*p2 <= '9'))
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[2] = atoi(p1);
+       if ((string = pc->read_vmcoreinfo("OSRELEASE"))) {
+               parse_kernel_version(string);
+               free(string);
        }
-       free(string);
        return THIS_KERNEL_VERSION;
 }

diff --git a/defs.h b/defs.h
index 5ee60f1..1925148 100644
--- a/defs.h
+++ b/defs.h
@@ -6032,6 +6032,8 @@ void clone_bt_info(struct bt_info *, struct bt_info *, struct task_context *);
 void dump_kernel_table(int);
 void dump_bt_info(struct bt_info *, char *where);
 void dump_log(int);
+void parse_kernel_version(char *);
+
 #define LOG_LEVEL(v) ((v) & 0x07)
 #define SHOW_LOG_LEVEL (0x1)
 #define SHOW_LOG_DICT  (0x2)
diff --git a/kernel.c b/kernel.c
index 2114700..988206b 100644
--- a/kernel.c
+++ b/kernel.c
@@ -104,6 +104,38 @@ static void check_vmcoreinfo(void);
 static int is_pvops_xen(void);
 static int get_linux_banner_from_vmlinux(char *, size_t);

+/*
+ * popuplate the global kernel table (kt) with kernel version
+ * information parsed from UTSNAME/OSRELEASE string
+ */
+void
+parse_kernel_version(char *str)
+{
+       char *p1, *p2, separator;
+
+       p1 = p2 = str;
+       while (*p2 != '.' && *p2 != '\0')
+               p2++;
+
+       *p2 = NULLCHAR;
+       kt->kernel_version[0] = atoi(p1);
+       p1 = ++p2;
+       while (*p2 != '.' && *p2 != '-' && *p2 != '\0')
+               p2++;
+
+       separator = *p2;
+       *p2 = NULLCHAR;
+       kt->kernel_version[1] = atoi(p1);
+
+       if (separator == '.') {
+               p1 = ++p2;
+               while ((*p2 >= '0') && (*p2 <= '9'))
+                       p2++;
+
+               *p2 = NULLCHAR;
+               kt->kernel_version[2] = atoi(p1);
+       }
+}

 /*
  *  Gather a few kernel basics.
@@ -112,7 +144,7 @@ void
 kernel_init()
 {
        int i, c;
-       char *p1, *p2, buf[BUFSIZE];
+       char buf[BUFSIZE];
        struct syment *sp1, *sp2;
        char *rqstruct;
        char *rq_timestamp_name = NULL;
@@ -270,28 +302,7 @@ kernel_init()
        if (buf[64])
                buf[64] = NULLCHAR;
        if (ascii_string(kt->utsname.release)) {
-               char separator;
-
-               p1 = p2 = buf;
-               while (*p2 != '.')
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[0] = atoi(p1);
-               p1 = ++p2;
-               while (*p2 != '.' && *p2 != '-' && *p2 != '\0')
-                       p2++;
-               separator = *p2;
-               *p2 = NULLCHAR;
-               kt->kernel_version[1] = atoi(p1);
-               *p2 = separator;
-               if (*p2 == '.') {
-                       p1 = ++p2;
-                       while ((*p2 >= '0') && (*p2 <= '9'))
-                               p2++;
-                       *p2 = NULLCHAR;
-                       kt->kernel_version[2] = atoi(p1);
-               } else
-                       kt->kernel_version[2] = 0;
+               parse_kernel_version(buf);

                if (CRASHDEBUG(1))
                        fprintf(fp, "base kernel version: %d.%d.%d\n",
@@ -10973,8 +10984,6 @@ void
 get_log_from_vmcoreinfo(char *file)
 {
        char *string;
-       char buf[BUFSIZE];
-       char *p1, *p2;
        struct vmcoreinfo_data *vmc = &kt->vmcoreinfo;

        if (!(pc->flags2 & VMCOREINFO))
@@ -10986,22 +10995,8 @@ get_log_from_vmcoreinfo(char *file)
        if ((string = pc->read_vmcoreinfo("OSRELEASE"))) {
                if (CRASHDEBUG(1))
                        fprintf(fp, "OSRELEASE: %s\n", string);
-               strcpy(buf, string);
-               p1 = p2 = buf;
-               while (*p2 != '.')
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[0] = atoi(p1);
-               p1 = ++p2;
-               while (*p2 != '.')
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[1] = atoi(p1);
-               p1 = ++p2;
-               while ((*p2 >= '0') && (*p2 <= '9'))
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[2] = atoi(p1);
+
+               parse_kernel_version(string);

                if (CRASHDEBUG(1))
                        fprintf(fp, "base kernel version: %d.%d.%d\n",
diff --git a/riscv64.c b/riscv64.c
index 6b9a688..066cf2c 100644
--- a/riscv64.c
+++ b/riscv64.c
@@ -259,33 +259,12 @@ riscv64_processor_speed(void)
 static unsigned long riscv64_get_kernel_version(void)
 {
        char *string;
-       char buf[BUFSIZE];
-       char *p1, *p2;

        if (THIS_KERNEL_VERSION)
                return THIS_KERNEL_VERSION;

-       string = pc->read_vmcoreinfo("OSRELEASE");
-       if (string) {
-               strcpy(buf, string);
-
-               p1 = p2 = buf;
-               while (*p2 != '.')
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[0] = atoi(p1);
-
-               p1 = ++p2;
-               while (*p2 != '.')
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[1] = atoi(p1);
-
-               p1 = ++p2;
-               while ((*p2 >= '0') && (*p2 <= '9'))
-                       p2++;
-               *p2 = NULLCHAR;
-               kt->kernel_version[2] = atoi(p1);
+       if ((string = pc->read_vmcoreinfo("OSRELEASE"))) {
+               parse_kernel_version(string);
                free(string);
        }
        return THIS_KERNEL_VERSION;
--
2.41.0