From patchwork Fri Mar 11 05:10:56 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jon Povey X-Patchwork-Id: 86377 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id D2119B6FAA for ; Fri, 11 Mar 2011 16:38:10 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 7DA432807B; Fri, 11 Mar 2011 06:38:06 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lVpDNdsIL1Fg; Fri, 11 Mar 2011 06:38:06 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id C410B2809E; Fri, 11 Mar 2011 06:38:04 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 6DB0D2809E for ; Fri, 11 Mar 2011 06:38:01 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id j92VGaKyINmr for ; Fri, 11 Mar 2011 06:37:59 +0100 (CET) X-Greylist: delayed 1607 seconds by postgrey-1.27 at theia; Fri, 11 Mar 2011 06:37:56 CET X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from smtp.sand.ocn.ne.jp (sand.ocn.ne.jp [122.28.30.136]) by theia.denx.de (Postfix) with ESMTP id 1FFC32807B for ; Fri, 11 Mar 2011 06:37:56 +0100 (CET) Received: from localhost.localdomain (p4129-ipbf6309marunouchi.tokyo.ocn.ne.jp [114.145.195.129]) by smtp.sand.ocn.ne.jp (Postfix) with ESMTP id 2A600236A; Fri, 11 Mar 2011 14:11:06 +0900 (JST) From: Jon Povey To: u-boot@lists.denx.de Date: Fri, 11 Mar 2011 14:10:56 +0900 Message-Id: <1299820256-29757-1-git-send-email-jon.povey@racelogic.co.uk> X-Mailer: git-send-email 1.6.3.3 Subject: [U-Boot] [PATCH] tools/env: fix redundant env flag comparison X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.9 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de This fixes two bugs with comparison of redundant environment flags on read. flag0 and flag1 in fw_env_open() were declared signed instead of unsigned char breaking BOOLEAN mode "== 0xFF" tests and in INCREMENTAL mode the wrong environment would be chosen where the flag values are 127 and 128 (either way round). With both flags over 128, both signs flipped and the logic worked by happy accident. Also there was a logic bug in the INCREMENTAL test (after signedness was fixed) in the case flag0=0, flag1=255, env 1 would be incorrectly chosen. Fix both of these. Signed-off-by: Jon Povey --- After my gripe yesterday, something more constructive: I confirmed these bugs with a little test program in C, can send that if anyone wants it. At a glance it looks like the main u-boot env handling does not have these bugs. tools/env/fw_env.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c index 8ff7052..315d7ce 100644 --- a/tools/env/fw_env.c +++ b/tools/env/fw_env.c @@ -1067,11 +1067,11 @@ static char *envmatch (char * s1, char * s2) int fw_env_open(void) { int crc0, crc0_ok; - char flag0; + unsigned char flag0; void *addr0; int crc1, crc1_ok; - char flag1; + unsigned char flag1; void *addr1; struct env_image_single *single; @@ -1182,14 +1182,13 @@ int fw_env_open(void) } break; case FLAG_INCREMENTAL: - if ((flag0 == 255 && flag1 == 0) || - flag1 > flag0) + if (flag0 == 255 && flag1 == 0) dev_current = 1; else if ((flag1 == 255 && flag0 == 0) || - flag0 > flag1) - dev_current = 0; - else /* flags are equal - almost impossible */ + flag0 >= flag1) dev_current = 0; + else /* flag1 > flag0 */ + dev_current = 1; break; default: fprintf (stderr, "Unknown flag scheme %u \n",