Re: [dpdk-dev] [PATCH 4/7] Move EAL common functions

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2015-01-05 10:56, Ravi Kerur:
> On Mon, Jan 5, 2015 at 7:59 AM, Thomas Monjalon <thomas.monjalon@6wind.com>
> 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.
> 
> <rk> 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.

-- 
Thomas