From patchwork Thu Nov 8 16:33:48 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Ravnborg X-Patchwork-Id: 197856 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id AAB892C01FA for ; Fri, 9 Nov 2012 04:29:04 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756304Ab2KHQdv (ORCPT ); Thu, 8 Nov 2012 11:33:51 -0500 Received: from smtp.snhosting.dk ([87.238.248.203]:38609 "EHLO smtp.domainteam.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756273Ab2KHQdv (ORCPT ); Thu, 8 Nov 2012 11:33:51 -0500 Received: from merkur.ravnborg.org (unknown [188.228.89.252]) by smtp.domainteam.dk (Postfix) with ESMTPA id 3FCCCF1910; Thu, 8 Nov 2012 17:33:49 +0100 (CET) Date: Thu, 8 Nov 2012 17:33:48 +0100 From: Sam Ravnborg To: Chen Gang Cc: David Miller , sparclinux@vger.kernel.org Subject: Re: [PATCH] arch/sparc: additional len check in loop for prom_getbootargs Message-ID: <20121108163348.GA7729@merkur.ravnborg.org> References: <509B29F3.2040006@asianux.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <509B29F3.2040006@asianux.com> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: sparclinux-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: sparclinux@vger.kernel.org Hi Cheng. On Thu, Nov 08, 2012 at 11:41:39AM +0800, Chen Gang wrote: > > when cp >= barg_buf + BARG_LEN-2, it only break internel loop (while) > but outside loop (for) still has effect, and "*cp++ = ' '" repeating > so need additional checking for it. > > > Signed-off-by: Chen Gang I wonder how you found this bug?!?! Anyway please consider this alternative fix: Adding the conditional inside the while loop makes the logic simpler. And the patch actually deletes more lines than it adds. And please take care to follow coding style too. In particular spaces around operators. The old code does not follow coding style - but this is no excuse. Note - the above is not even build tested! If you use the above code-snippet you can add my: Acked-by: Sam Ravnborg Sam --- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/sparc/prom/bootstr_32.c b/arch/sparc/prom/bootstr_32.c index f5ec32e..4ce602f 100644 --- a/arch/sparc/prom/bootstr_32.c +++ b/arch/sparc/prom/bootstr_32.c @@ -31,14 +31,10 @@ prom_getbootargs(void) arg = (*(romvec->pv_v0bootargs))->argv[iter]; if (arg == NULL) break; - while(*arg != 0) { - /* Leave place for space and null. */ - if(cp >= barg_buf + BARG_LEN-2){ - /* We might issue a warning here. */ - break; - } + while (*arg != 0 && cp < (barg_buf + BARG_LEN - 2)) *cp++ = *arg++; - } + + /* Append trailing space + null */ *cp++ = ' '; } *cp = 0;