Re: [uml-devel] sysrq patch

[Jeff Dike <jdike@addtoit.com>]

On Mon, Sep 26, 2005 at 02:05:13PM -0400, Allan Graves wrote:
> Let me know what you want changed, jeff, 

OK, you asked for it :-)

> i set vim to hardtabs, i hope 
> the white space comes out okay, don't want to get you in trouble again.  

The tabs look OK this time, thanks.

> i ran this in both frame pointer and no frame pointer mode.

+union uml_pt_regs *get_thread_regs(union uml_pt_regs *uml_regs, void *buffer)
+{
	...
+       return uml_regs;
+}

This should just be a void, there's usually no sense in returning an argument.
I see why you do it this way, but see my comments later.

	//jmp_buf * jmpbuf=(jmp_buf  *)buffer;

Just get rid of commented code - act like you mean it :-)

	+int stop_here=0;

You should have gotten rid of that.

+               printk("ASM\n");

What's that?

+#ifdef CONFIG_MODE_SKAS
+
+#define KSTK_EIP(tsk) UPT_REG(get_thread_regs(&tsk->thread.sysrq_regs, tsk->thread.mode.skas.switch_buf), EIP);
+#define KSTK_ESP(tsk) UPT_REG(get_thread_regs(&tsk->thread.sysrq_regs, tsk->thread.mode.skas.switch_buf), UESP);
+#define KSTK_EBP(tsk) UPT_REG(get_thread_regs(&tsk->thread.sysrq_regs, tsk->thread.mode.skas.switch_buf), EBP);
+
+#else

With this, I would prefer something like 

static inline unsigned long KSTK_EIP(struct task_struct *task)
{
	union uml_pt_regs regs;

	get_thread_regs(&regs, task->thread.mode.skas.switch_buf);
	return(UPT_REG(regs, EIP));
}

No need to make the thread structure any larger than it needs to be.  Plus
the typechecking is better with a function than a macro.

Also, is KSTK_EBP needed?  It wasn't defined before, and yet UML still
linked.

And I would make the tt mode versions return obviously bogus values
(0x00badbad, maybe :-), rather than non-obviously bogus values.

				Jeff


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel