> Subject: Re: [dpdk-dev] [PATCH v8 1/9] eal: move OS common functions to
> single file
>
> Can we reduce the scope in the title?
> This patch is about EAL config in general, so I would reword it to:
> eal: move OS common config objects
>
Agree, will change in v9.
> 22/06/2020 09:55, tal...@mellanox.com:
> > +#include <rte_os.h>
> > +
> > +#include <eal_private.h>
> > +#include <eal_memcfg.h>
>
> Local files, like eal_*, are usually included with "".
>
> [...]
> > +/* Allow the application to print its usage message too if set */
> > +static rte_usage_hook_t rte_application_usage_hook;
>
> This is not config-related.
> Could it be in a separate patch?
> Moved to lib/librte_eal/common/eal_common_options.c?
>
Agree, will change in v9.
> [...]
> > +void
> > +rte_eal_set_runtime_dir(char *run_dir, size_t size) {
> > + strncpy(runtime_dir, run_dir, size); }
>
> This function should be private to EAL, no rte_ prefix.
>
> [...]
> > +/* Return a pointer to theinternal configuration structure */
>
> Space missing
>
> > +struct internal_config *
> > +rte_eal_get_internal_configuration(void)
> > +{
> > + return &internal_config;
> > +}
>
> This function should be private to EAL, no rte_ prefix.
>
When creating the new functions I based the pattern on the already existing
function
rte_eal_get_configuration() which encapsulates the formally global rte_config.
Do we want to remove the rte_ prefix from this function as well?
> [...]
> > --- a/lib/librte_eal/include/rte_eal.h
> > +++ b/lib/librte_eal/include/rte_eal.h
> > +void
> > +rte_eal_set_runtime_dir(char *run_dir, size_t size);
> > +
> > +void
> > +rte_eal_config_remap(void *mem_cfg_addr);
> > +
> > +struct internal_config *
> > +rte_eal_get_internal_configuration(void);
> > +
> > +rte_usage_hook_t *
> > +rte_eal_get_application_usage_hook(void);
>
> If some new function must be added in the API, they must be clearly
> documented with doxygen.
> But as part of this refactoring, I guess we should have zero new API.
> Probably that those 4 functions are candidate to internal header files.
>
