Re: [PATCH] kgdbts: unify/generalize gdb breakpoint adjustment

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 4 Jun 2009 21:50:49 -0400 Mike Frysinger <vapier.adi@gmail.com> wrote:

> On Thu, Jun 4, 2009 at 21:04, Andrew Morton wrote:
> > On Thu, 4 Jun 2009 20:55:40 -0400 Mike Frysinger wrote:
> >> On Thu, Jun 4, 2009 at 20:50, Andrew Morton wrote:
> >> > On Tue, __2 Jun 2009 03:17:30 -0400 Mike Frysinger wrote:
> >> >> + __ __ instruction_pointer(&kgdbts_regs) += offset;
> >> >
> >> > instruction_pointer() cannot be used as an lvalue, thankfully.
> >> >
> >> > x86_64:
> >> >
> >> > drivers/misc/kgdbts.c: In function 'check_and_rewind_pc':
> >> > drivers/misc/kgdbts.c:306: error: invalid lvalue in assignment
> >>
> >> should be easy to fix:
> >> --- a/arch/x86/include/asm/ptrace.h
> >> +++ b/arch/x86/include/asm/ptrace.h
> >> @@ -236,10 +236,7 @@
> >> __#endif
> >> __}
> >>
> >> -static inline unsigned long instruction_pointer(struct pt_regs *regs)
> >> -{
> >> - __ return regs->ip;
> >> -}
> >> +#define instruction_pointer(regs) ((regs)->ip)
> >>
> >> __static inline unsigned long frame_pointer(struct pt_regs *regs)
> >> __{
> >
> > argh, that's soooooo tasteless. __Look, this:
> >
> > __ __ __ __instruction_pointer(&kgdbts_regs) += offset;
> >
> > is just daft. __It's not C!

Gets frustrating when I say correct things and your first reaction is
to argue.

> it is C.  taste is one thing, but valid C is still valid C.

It can be compiled.  But it is not idiomatically C.  You can implement
any level of stupidity with the preprocessor and compile the result. 
That doesn't make it desirable.

Plus all the other things I said which you ignored.  Plus the
conversion to a macros weakens typechecking.

> > It makes no sense to define something which
> > looks like a function and to then assign values to it. __It means that
> > instruction_pointer() _must_ be implemented as a macro, violating basic
> > concepts of encapsualtion/layering/hiding/etc.
> >
> > Doing
> >
> > void instruction_pointer_set(struct pt_regs *regs, some_suitable_type val);
> >
> > will save many vomit bags.
> 
> and force everyone to implement the same copy & paste set of get/set
> modifiers ?  x86 is the only one where instruction_pointer() isnt a
> define.

Please, look at ia64:

# define instruction_pointer(regs) ((regs)->cr_iip + ia64_psr(regs)->ri)