This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]Systemtap IA64 Runtime support


On Fri, Oct 28, 2005 at 02:18:12PM -0400, Frank Ch. Eigler wrote:
Hi Frank,
	Thanks for taking a look at my code, I have inlined my replies to
your comments. 

I will refresh my patch and send it again once you confirm my comments.

Thanks,
Anil

> 
> anil.s.keshavamurthy wrote:
> 
> > [...]
> > --- src.orig/runtime/copy.c
> > +++ src/runtime/copy.c
> > @@ -1,3 +1,12 @@
> > +/* Copy from user space functions
> > + * Copyright (C) 2005 Intel Corporation.
> > + *
> > + * This file is part of systemtap, and is free software.  You can
> > + * redistribute it and/or modify it under the terms of the GNU General
> > + * Public License (GPL); either version 2, or (at your option) any
> > + * later version.
> > + */
> (There seems to have been an oversight in attaching Red Hat copyright
> notices to some of the runtime files.)

ARe you asking me to remove the above lines in all the files?
The reason I copied the above was that some files had this GPL statement and
some files didn't.
Also are you saying not keep even "Copyright (C) 2005 Intel Corporation"


> 
> 
> >  #elif defined (__powerpc64__)
> >  #define __stp_strncpy_from_user(dst,src,count,res) \
> >  	do { res = __strncpy_from_user(dst, src, count); } while(0)
> > +#elif defined (__ia64__)
> > +#define __stp_strncpy_from_user(dst,src,count,res) \
> > +	do { res = __strncpy_from_user(dst, src, count); } while(0)
> >  #endif
> 
> Why not  "#elif defined (__powerpc64__) || defined (__ia64__)?
No problem. I can do this.

> 
> 
> > [...]
> > @@ -25,10 +34,9 @@ String _stp_symbol_sprint (String str, u
> > -	_stp_sprintf (str, "0x%lx", address);
> > -
> >  	if (name) {		
> > -		if (modname)
> > +		_stp_sprintf (str, "0x%lx", address);
> 
> Why is this part now conditional on <name> ?
Oh well..Initially I had thought of dumping the address only if it has a
corresponding symbol name. I think original code was right where it prints
the address irrespective of its symbol name. I will revert back.

> 
> > [...]
> > +	// In IA64 platform function probe point is set at its
> > +	// entry point rather than prologue end pointer, and in
> > +	// IA64 prologue end point also is not computed in such way
> 
> I probably missed the discussion, but why exactly is this?  Are all
> the parameters available at the debuginfo-identified location list
> right at this entry address?
On Ia64 platform, I found probe address of function entry translated by
systemtap was one bundle more than the actual address. However, when
queried from entrypc rather than prolouge-end of the function we got the
right address translation. Let me know if you are ok with this change and
I will reword my comments.

> 
> 
> > [...]
> > -      s.op->newline() << "#define MAXSTRINGLEN 128";
> > +      s.op->newline() << "#define MAXSTRINGLEN 256";
> 
> Why?
Humm..I think this can be reverted back.




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]