[Crash-utility] Re: [PATCH 1/2] Add a new helper function get_value_vmcore

[Shijie Huang <shijie@xxxxxxxxxxxxxxxxxxxxxxxxxx>]

在 2023/11/30 15:22, HAGIO KAZUHITO(萩尾 一仁) 写道:
> On 2023/11/28 12:41, Huang Shijie wrote:
> > Add get_value_vmcore() to get the symbol value for @name.
> >
> > Also add macro GET_SYM to simplify the code.
> >
> > Signed-off-by: Huang Shijie <shijie@xxxxxxxxxxxxxxxxxxxxxx>
> > ---
> >    defs.h   |  1 +
> >    kernel.c | 85 +++++++++++++++++++-------------------------------------
> >    2 files changed, 30 insertions(+), 56 deletions(-)
> >
> > diff --git a/defs.h b/defs.h
> > index 1fe2d0b..eec7b3e 100644
> > --- a/defs.h
> > +++ b/defs.h
> > @@ -6055,6 +6055,7 @@ int hide_offline_cpu(int);
> >    int get_highest_cpu_online(void);
> >    int get_highest_cpu_present(void);
> >    int get_cpus_to_display(void);
> > +bool get_value_vmcore(const char *name, ulong *v);
> >    void get_log_from_vmcoreinfo(char *file);
> >    int in_cpu_map(int, int);
> >    void paravirt_init(void);
> > diff --git a/kernel.c b/kernel.c
> > index 6dcf414..caf6149 100644
> > --- a/kernel.c
> > +++ b/kernel.c
> > @@ -104,6 +104,20 @@ static void check_vmcoreinfo(void);
> >    static int is_pvops_xen(void);
> >    static int get_linux_banner_from_vmlinux(char *, size_t);
> >    
> > +/* Return TRUE if we succeed, return FALSE on failure. */
> > +bool
> > +get_value_vmcore(const char *name, ulong *v)
> "get_value_vmcoreinfo" is better to clarify.

What about get_symbol_value_vmcoreinfo()?

We can use "htol" to do the convert for symbol, but htol does not work 
for "NUMBER()"..


> > +{
> > +	char *string = pc->read_vmcoreinfo(name);
> > +
> > +	if (!string)
> > +		return FALSE;
> > +
> > +	*v = htol(string, RETURN_ON_ERROR, NULL);

> > +	return TRUE;
> > +}
> > +
> >    /*
> >     * popuplate the global kernel table (kt) with kernel version
> >     * information parsed from UTSNAME/OSRELEASE string
> > @@ -10984,6 +10998,12 @@ hypervisor_init(void)
> >    		fprintf(fp, "hypervisor: %s\n", kt->hypervisor);
> >    }
> >    
> > +#define GET_SYM(s,v)	\
> How about "GET_SYMBOL" and

no problem.


> > +	if (get_value_vmcore((s), &(v))) {	\
> adding "SYMBOL(" and ")" here?

it is not easy to add "SYMBOL" here.

I prefer to add to get_symbol_value_vmcoreinfo().

> > +		if (CRASHDEBUG(1))		\
> > +			fprintf(fp, s ": %lx\n", v);	\
> > +	}
> > +
> >    /*
> >     *  Get and display the kernel log buffer using the vmcoreinfo
> >     *  data alone without the vmlinux file.
> > @@ -11024,62 +11044,15 @@ get_log_from_vmcoreinfo(char *file)
> >    	} else
> >    		error(FATAL, "VMCOREINFO: cannot determine page size\n");
> >    
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf)"))) {
> > -		vmc->log_buf_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(log_buf): %lx\n",
> > -				vmc->log_buf_SYMBOL);
> > -		free(string);
> > -	}
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(log_end)"))) {
> > -		vmc->log_end_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(log_end): %lx\n",
> > -				vmc->log_end_SYMBOL);
> > -		free(string);
> > -	}
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(log_buf_len)"))) {
> > -		vmc->log_buf_len_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(log_buf_len): %lx\n",
> > -				vmc->log_buf_len_SYMBOL);
> > -		free(string);
> > -	}
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(logged_chars)"))) {
> > -		vmc->logged_chars_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(logged_chars): %lx\n",
> > -				vmc->logged_chars_SYMBOL);
> > -		free(string);
> > -	}
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(log_first_idx)"))) {
> > -		vmc->log_first_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(log_first_idx): %lx\n",
> > -				vmc->log_first_idx_SYMBOL);
> > -		free(string);
> > -	}
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(log_next_idx)"))) {
> > -		vmc->log_next_idx_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(log_next_idx): %lx\n",
> > -				vmc->log_next_idx_SYMBOL);
> > -		free(string);
> > -	}
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(phys_base)"))) {
> > -		vmc->phys_base_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(phys_base): %lx\n",
> > -				vmc->phys_base_SYMBOL);
> > -		free(string);
> > -	}
> > -	if ((string = pc->read_vmcoreinfo("SYMBOL(_stext)"))) {
> > -		vmc->_stext_SYMBOL = htol(string, RETURN_ON_ERROR, NULL);
> > -		if (CRASHDEBUG(1))
> > -			fprintf(fp, "SYMBOL(_stext): %lx\n",
> > -				vmc->_stext_SYMBOL);
> > -		free(string);
> > -	}
> > +	GET_SYM("SYMBOL(log_buf)", vmc->log_buf_SYMBOL);
> then GET_SYMBOL("log_buf", vmc->log_buf_SYMBOL);

okay.

Thanks

Huang Shijie
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki