On 2023/08/14 22:42, Rafael Aquini wrote:
> On Mon, Aug 14, 2023 at 09:41:12AM -0400, Rafael Aquini wrote:
>> 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.
>>
> 
> this changeset is also at https://github.com/crash-utility/crash/pull/151
>   
>> Signed-off-by: Rafael Aquini <aqu...@redhat.com>

Good to see you again :)

Thank you for the clean-up, looks good to me.

Acked-by: Kazuhito Hagio <k-hagio...@nec.com>

Thanks,
Kazu


>> ---
>>   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
>>
> 
> --
> Crash-utility mailing list
> Crash-utility@redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility
> Contribution Guidelines: https://github.com/crash-utility/crash/wiki
--
Crash-utility mailing list
Crash-utility@redhat.com
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to