Re: [PATCH] powerpc/xmon: symbol lookup length fixed

[Michael Ellerman <mpe@ellerman.id.au>]

Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com> writes:
> On Thu, Oct 24, 2024 at 10:42:12AM +0530, Mukesh Kumar Chaurasiya wrote:
>> On Thu, Oct 24, 2024 at 12:00:53PM +1100, Michael Ellerman wrote:
>> > Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com> writes:
>> > > Currently xmon cannot lookup symbol beyond 64 characters in some cases.
>> > 
>> > Can you mention which commands? It looks like it's "ls" and "lp".
>> Sure.
>> > 
>> > > Fix this by using KSYM_NAME_LEN instead of fixed 64 characters.
>> > >
>> > > Signed-off-by: Mukesh Kumar Chaurasiya <mchauras@linux.ibm.com>
>> > > ---
>> > >  arch/powerpc/xmon/xmon.c | 6 +++---
>> > >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> > > index e6cddbb2305f..22b8b5cc4df0 100644
>> > > --- a/arch/powerpc/xmon/xmon.c
>> > > +++ b/arch/powerpc/xmon/xmon.c
>> > > @@ -3662,7 +3662,7 @@ symbol_lookup(void)
>> > >  	int type = inchar();
>> > >  	unsigned long addr, cpu;
>> > >  	void __percpu *ptr = NULL;
>> > > -	static char tmp[64];
>> > > +	static char tmp[KSYM_NAME_LEN];
>> >   
>> > I think you could use the existing tmpstr buffer.
>> > 
>> > It is global so it's a little hard to track down all the users, but I
>> > think it's only used briefly in get_function_bounds(),
>> > xmon_print_symbol() and scanhex(). ie. none of the uses persist across
>> > function calls.
>> > 
>> > We don't want to have two 512 byte static arrays lying around if we can
>> > get by with one.
>> > 
>> > cheers
>> Sure.

> For now i don't think so we can get by with only one. There is a lookup being
> done when the scanhex is using the tmpstr for another lookup.

Yeah I see it, when handling `lp <symbol> <cpu>`.

That's a bit annoying because for that command the CPU number is only
meant to be a number, it never makes sense to do a $symbol lookup there.

But I agree with the way the code is structured it's best not to reuse
tmpstr there.

cheers