From: Wolfgang Denk <wd@denx.de>
Date: Sat, 17 Nov 2007 00:26:10 +0100
Subject: [U-Boot-Users] [PATCH 1/2] Fix warning differ in signedness on
	NetSetTimeout in net/net.c and net/nfs.c
In-Reply-To: Your message of "Wed, 07 Nov 2007 08:19:20 +0100."
	<1194419961-21822-2-git-send-email-plagnioj@jcrosoft.com>
Message-ID: <20071116232610.2622424888@gemini.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

In message <1194419961-21822-2-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> diff --git a/net/net.c b/net/net.c
> index cde2680..f3c70c5 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -598,7 +598,7 @@ void NetStartAgain (void)
>  	if (NetRestartWrap) {
>  		NetRestartWrap = 0;
>  		if (NetDevExists && !once) {
> -			NetSetTimeout (10 * CFG_HZ, startAgainTimeout);
> +			NetSetTimeout ((ulong)10 * CFG_HZ, startAgainTimeout);
>  			NetSetHandler (startAgainHandler);
>  		} else {
>  			NetState = NETLOOP_FAIL;

This doesn't look right to me.

Instead of "(ulong)10 * CFG_HZ" we should probably write "10UL * CFG_HZ"

And, why do we cast the first operand to "ulong" - and not the second
one, or the second one, too, or the result?

Finally, this is not the only place where this construct is used:

-> grep -n CFG_HZ net/net.c
253:    if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ) {
592:    NetSetTimeout (10 * CFG_HZ, startAgainTimeout);
601:                    NetSetTimeout (10 * CFG_HZ, startAgainTimeout);
777:    NetSetTimeout (10 * CFG_HZ, PingTimeout);
800:#define CDP_TIMEOUT                 (CFG_HZ/4)      /* one packet every 250ms */

> diff --git a/net/nfs.c b/net/nfs.c
> index 1b27c97..18b00cc 100644
> --- a/net/nfs.c
> +++ b/net/nfs.c
> @@ -669,7 +669,7 @@ NfsHandler (uchar *pkt, unsigned dest, unsigned src, unsigned len)
>  
>  	case STATE_READ_REQ:
>  		rlen = nfs_read_reply (pkt, len);
> -		NetSetTimeout (NFS_TIMEOUT * CFG_HZ, NfsTimeout);
> +		NetSetTimeout ((ulong)NFS_TIMEOUT * CFG_HZ, NfsTimeout);
>  		if (rlen > 0) {
>  			nfs_offset += rlen;
>  			NfsSend ();
> @@ -758,7 +758,7 @@ NfsStart (void)
>  	printf ("\nLoad address: 0x%lx\n"
>  		"Loading: *\b", load_addr);
>  
> -	NetSetTimeout (NFS_TIMEOUT * CFG_HZ, NfsTimeout);
> +	NetSetTimeout ((ulong)NFS_TIMEOUT * CFG_HZ, NfsTimeout);
>  	NetSetHandler (NfsHandler);
>  
>  	NfsTimeoutCount = 0;

The first two questions from above apply here, too.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Status quo. Latin for "the mess we're in."