From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ravi Kerur Subject: Re: [PATCH 4/7] Move EAL common functions Date: Tue, 6 Jan 2015 09:35:20 -0800 Message-ID: References: <1419521597-31978-1-git-send-email-rkerur@gmail.com> <20293089.lIc3yRvdMZ@xps13> <3095360.cfZ4UIMKPn@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Cc: "dev-VfR2kkLFssw@public.gmane.org" To: Thomas Monjalon Return-path: In-Reply-To: <3095360.cfZ4UIMKPn@xps13> 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" Inline On Mon, Jan 5, 2015 at 12:38 PM, Thomas Monjalon wrote: > 2015-01-05 10:56, Ravi Kerur: > > On Mon, Jan 5, 2015 at 7:59 AM, Thomas Monjalon < > thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> > > wrote: > > > 2014-12-25 10:33, Ravi Kerur: > > > > Move common functions in eal.c to librte_eal/common directory. > > > [...] > > > > lib/librte_eal/common/eal_common.c | 328 > ++++++++++++++++++++++++++++++++++ > > > > lib/librte_eal/common/eal_externs.h | 42 +++++ > > > > > > I don't agree with these new files. > > > We must try to keep a semantic organization. The file > eal_common_options.c > > > would be better for option-related functions. > > > Maybe that the split between system config, runtime config and internal > > > config > > > must be reworked. > > > > > > By the way, it would be nice to avoid extern variables. > > > > I have taken care of your comments and will generate v4 patch. > > Please do not forget v4 word and changelog when sending patches. > Check http://dpdk.org/dev#send > > > Currently I have moved common functions in eal.c into > > "eal_common_system_options.c" file. Are you suggesting that we further > > divide "eal_common_options.c" and "eal_common_system_options.c(new file > > added)" into 3 separate files i.e. > > > > eal_common_system_options.c > > eal_common_runtime_options.c > > eal_common_internal_options.c > > You are already doing big changes. So let's iterate with existing files and > avoid creating new ones. > eal_common_options.c must be kept. But if some code is not really related > to > runtime options, we could consider adding a new file eal_common_config.c, > not > sure about this one. > > -- > i have divided eal_common.c (new file which was added in v1 PATCH) into eal_common_system.c and eal_common_runtime.c and have moved appropriate functions into it. If file names are not appropriate or changes are not worth doing the split, I will merge them back into eal_common_config.c. Thanks. > Thomas >