From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Sat, 07 Feb 2004 00:34:13 +0100
Subject: [U-Boot-Users] PATCH: itest - an integer compare function 
In-Reply-To: Your message of "Tue, 06 Jan 2004 17:11:10 +1300."
             <3FFA355E.4090306@tait.co.nz> 
Message-ID: <20040206233418.9B73DC1096@atlas.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

Dear Robin,

in message <3FFA355E.4090306@tait.co.nz> you wrote:
> 
> Introduce a new command to be used in conditional expressions (such as 
> hush 'if') to test the value of a memory location.
> Supports -eq, -ne, -gt, -lt, -ge, -le
> Allows scalars or memory contents (value prefixed by '*')
> Makes use of command data width expression of .l, .wm .b for 4, 2 or 1 
> byte wide comparisons
> e.g.
> if itest.l *40000 -eq 12345678; then bootm 41000; else bootm 80000; fi

Sorry, but I don;t want to add the code as is.

May I suggest you rewrite it a bit?


> +	switch (op) {
> +	case EQ:
> +		return (l == r);
> +	case NE:
> +		return (l != r);
> +	case LT:
> +		return (l < r);
> +	case GT:
> +		return (l > r);
> +	case LE:
> +		return (l <= r);
> +	case GE:
> +		return (l >= r);
> +	}
> +
> +	return (FALSE);
> +}
> +
> +
> +int binary_test(char *op, char *arg1, char *arg2, int w)
> +{
> +
> +	if (op[2] == 't') {
> +		switch (op[1]) {
> +		case 'l':
> +			return (arithcomp(arg1, arg2, LT, w));	/* -lt */
> +		case 'g':
> +			return (arithcomp(arg1, arg2, GT, w));	/* -gt */
> +		}
> +	} else if (op[1] == 'e') {
> +		switch (op[2]) {
> +		case 'q':
> +			return (arithcomp(arg1, arg2, EQ, w));	/* -eq */
> +		}
> +	} else if (op[2] == 'e') {
> +		switch (op[1]) {
> +		case 'n':
> +			return (arithcomp(arg1, arg2, NE, w));	/* -ne */
> +		case 'g':
> +			return (arithcomp(arg1, arg2, GE, w));	/* -ge */
> +		case 'l':
> +			return (arithcomp(arg1, arg2, LE, w));	/* -le */
> +		}
> +	}

Instead of all the nested tests we could use an  array  with  command
names and table lookup - which would also allow to use operators like
'<'  or ">=", and I also expect the total memory footprint to be even
smaller.

Also, instead of just returning FALSE we should  at  least  print  an
error message for invalid input / operators.

Finally, maybe you even want to add string compare :-)

Best regards,

Wolfgang Denk

-- 
See us @ Embedded World, Nuremberg, Feb 17 - 19,  Hall 12.0 Booth 440
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
"We have the right to survive!"
"Not be killing others."
	-- Deela and Kirk, "Wink of An Eye", stardate 5710.5