From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Subject: Re: [PATCH v5] eal_common_cpuflags: Fix %rbx corruption, and simplify the code Date: Wed, 16 Apr 2014 08:53:42 -0400 Message-ID: <20140416125342.GA11887@localhost.localdomain> References: <20140416104955.GA26829@hmsreliant.think-freely.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev-VfR2kkLFssw@public.gmane.org" To: "De Lara Guarch, Pablo" Return-path: Content-Disposition: inline In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" On Wed, Apr 16, 2014 at 11:09:46AM +0000, De Lara Guarch, Pablo wrote: > Hi Neil, > > >Oh wow, yes, the if conditionals definately should be checked for each iteration of the for loop. Good eye. > > >Still though, seems like a bug in gcc to check the state of the loop index on exit, when its never used to index the array at that value. Seems a bit like this bug: > >http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45180 > > >Thomas I'll send a patch to fix this directly Neil > > Actually, that index may be used in cause last "ret" value is 0: > > if (!ret) { > fprintf(stderr, > "ERROR: This system does not support \"%s\".\n" > "Please check that RTE_MACHINE is set correctly.\n", > cpu_feature_table[compile_time_flags[i]].name); > exit(1); > } > Ah, well there you have it. Thank you. Either way though, those two error checks need to be encased in brackets. It appears I missed it in my previous patch as the for loop only had one if, and so didn't need additional brackets, and I extended the check, requiring brackets that I never added. apologies. I've sent a patch for review Neil > Thanks, > > Pablo de Lara > DPDK SW Engineer > > -------------------------------------------------------------- > Intel Shannon Limited > Registered in Ireland > Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare > > > > >