Re: [PATCH 2.6] clean-up: fixes "comparison between signed andunsigned"

[Jesper Juhl]

On Mon, 6 Dec 2004, Riina Kikas wrote:

> This patch fixes warning "comparison between signed and unsigned"
> occuring on line 308
> 
> Signed-off-by: Riina Kikas <Riina.Kikas@xxxxxxx>
> 
> --- a/arch/i386/mm/fault.c	2004-12-02 21:30:30.000000000 +0000
> +++ b/arch/i386/mm/fault.c	2004-12-02 21:30:59.000000000 +0000
> @@ -302,7 +302,13 @@
>  		 * pusha) doing post-decrement on the stack and that
>  		 * doesn't show up until later..
>  		 */
> -		if (address + 32 < regs->esp)
> +		unsigned long regs_esp;
> +		if (regs->esp < 0) {
> +			regs_esp = 0;
> +		} else {
> +			regs_esp = regs->esp;
> +		}
> +		if (address + 32 < regs_esp)
>  			goto bad_area;
>  	}
>  	if (expand_stack(vma, address))

This seems a bit silly. If the stack pointer (esp) is ever negative that's 
clearly a bug somewhere. So instead of testing it for <0 and then setting 
your regs_esp variable to 0 it would make more sense to me to just 
BUG_ON(regs->esp < 0) or something, if you want to do anything at all. And 
if you want to silence the warning a exlicit cast to unsigned long should 
do I'd say, but I have a feeling the best thing is to just leave that 
warning alone, the code seems to be fine.

Also, your declaration of regs_esp interspaced with code (which is allowed 
by C99 but not C89) will break on C89 compilers, some of which are still 
supported - such as gcc 2.95.3 for instance.


-- 
Jesper Juhl


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/