This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH -rc] [BUGFIX] x86: fix kernel_trap_sp()
On Tue, 12 May 2009, Ingo Molnar wrote:
>
> yeah, this is cleaner and probably a tad faster.
Well, I don't care about the 'faster' part per se, but I could actually
see some case where a kernel-only version did some pointer dereference
that was invalid for the user version, and could oops, so putting it
inside the code that explicitly tests that it's not user-or-vm seems like
conceptually the right thing to do.
Of course, in this case it's the other way around - it's the non-kernel
version that does a dereference, and it just so happens to be safe (but
return an invalid pointer) when the trap was in kernel mode. So the
argument is pretty theoretical, but I think it's cleaner.
> One small detail:
>
> > + return (unsigned long)(®s->sp);
>
> the original commit had:
>
> > + return (unsigned long)®s->sp;
Ok, that's just because I didn't actually apply the original patch, I
just rewrote it entirely, and for some reason I put parenthesis around the
expression. You're right that it doesn't matter, and either is fine. I
don't really care, I suspect I add the parenthesis just because I don't
even want to have to think about the proper operator precedence rules.
So pick whichever version.
Linus