1
1

- Code review of the registration / lookup code with George

- Various fixes / cleanups as a result of the code review
  George makes a good point that we could consolidate a lot of the
  malloc's / strdup's in this code for platforms where we do not have
  a lot of memory (i.e., add up all the memory required and make one
  big malloc vs. lots of little mallocs).  This is something for
  post-first-release. 
- Convert another data structure to the OBJ_* interface.
- Split the internal prototypes and struct definitions into their own
  header file that won't be included anywhere else in Open MPI (except
  ompi_info).

This commit was SVN r1692.
Этот коммит содержится в:
Jeff Squyres 2004-07-13 20:24:26 +00:00
родитель a35dc35d09
Коммит 2aa5f92fca
4 изменённых файлов: 258 добавлений и 178 удалений

Просмотреть файл

@ -17,6 +17,7 @@ headers = \
base.h \
mca_base_module_exchange.h \
mca_base_param.h \
mca_base_param_internal.h \
mca_base_msgbuf.h \
mca_base_msgbuf_internal.h

Просмотреть файл

@ -14,13 +14,14 @@
#include "attribute/attribute.h"
#include "mca/mca.h"
#include "mca/base/mca_base_param.h"
#include "mca/base/mca_base_param_internal.h"
/*
* Public variables
*
* This variable is public, but not advertised in mca_base_param.h.
* It's only public so that ompi_info can see it. The relevant module
* It's only public so that ompi_info can see it. The relevant component
* in ompi_info will provide an extern to see this variable.
*/
ompi_value_array_t mca_base_params;
@ -36,21 +37,29 @@ static bool initialized = false;
/*
* local functions
*/
static int param_register(const char *type_name, const char *module_name,
static int param_register(const char *type_name, const char *component_name,
const char *param_name,
const char *mca_param_name,
mca_base_param_type_t type,
mca_base_param_storage_t *default_value);
static bool param_lookup(int index, mca_base_param_storage_t *storage,
ompi_hash_table_t *attrs);
static void param_free(mca_base_param_t *p);
static void param_constructor(mca_base_param_t *p);
static void param_destructor(mca_base_param_t *p);
/*
* Make the class instance for mca_base_param_t
*/
OBJ_CLASS_INSTANCE(mca_base_param_t, ompi_object_t,
param_constructor, param_destructor);
/*
* Register an integer MCA parameter
*/
int mca_base_param_register_int(const char *type_name,
const char *module_name,
const char *component_name,
const char *param_name,
const char *mca_param_name,
int default_value)
@ -58,7 +67,7 @@ int mca_base_param_register_int(const char *type_name,
mca_base_param_storage_t storage;
storage.intval = default_value;
return param_register(type_name, module_name, param_name, mca_param_name,
return param_register(type_name, component_name, param_name, mca_param_name,
MCA_BASE_PARAM_TYPE_INT, &storage);
}
@ -67,7 +76,7 @@ int mca_base_param_register_int(const char *type_name,
* Register a string MCA parameter.
*/
int mca_base_param_register_string(const char *type_name,
const char *module_name,
const char *component_name,
const char *param_name,
const char *mca_param_name,
const char *default_value)
@ -78,7 +87,7 @@ int mca_base_param_register_string(const char *type_name,
} else {
storage.stringval = NULL;
}
return param_register(type_name, module_name, param_name, mca_param_name,
return param_register(type_name, component_name, param_name, mca_param_name,
MCA_BASE_PARAM_TYPE_STRING, &storage);
}
@ -177,7 +186,7 @@ int mca_base_param_kv_lookup_string(int index, ompi_hash_table_t *attrs,
/*
* Find the index for an MCA parameter based on its names.
*/
int mca_base_param_find(const char *type_name, const char *module_name,
int mca_base_param_find(const char *type_name, const char *component_name,
const char *param_name)
{
size_t i, size;
@ -188,21 +197,23 @@ int mca_base_param_find(const char *type_name, const char *module_name,
if (!initialized) {
return OMPI_ERROR;
}
if (NULL == type_name || NULL == param_name) {
if (NULL == type_name) {
return OMPI_ERROR;
}
/* Loop through looking for a parameter of a given
type/module/param */
type/component/param */
size = ompi_value_array_get_size(&mca_base_params);
array = OMPI_VALUE_ARRAY_GET_BASE(&mca_base_params, mca_base_param_t);
for (i = 0; i < size; ++i) {
if (0 == strcmp(type_name, array[i].mbp_type_name) &&
((NULL == module_name && NULL == array[i].mbp_module_name) ||
(NULL != module_name && NULL != array[i].mbp_module_name &&
0 == strcmp(module_name, array[i].mbp_module_name))) &&
0 == strcmp(param_name, array[i].mbp_param_name)) {
((NULL == component_name && NULL == array[i].mbp_component_name) ||
(NULL != component_name && NULL != array[i].mbp_component_name &&
0 == strcmp(component_name, array[i].mbp_component_name))) &&
((NULL == param_name && NULL == array[i].mbp_param_name) ||
(NULL != param_name && NULL != array[i].mbp_param_name &&
0 == strcmp(param_name, array[i].mbp_param_name)))) {
return i;
}
}
@ -222,9 +233,12 @@ int mca_base_param_finalize(void)
mca_base_param_t *array;
if (initialized) {
/* This is slow, but effective :-) */
array = OMPI_VALUE_ARRAY_GET_BASE(&mca_base_params, mca_base_param_t);
while (0 < ompi_value_array_get_size(&mca_base_params)) {
param_free(&array[0]);
OBJ_DESTRUCT(&array[0]);
ompi_value_array_remove_item(&mca_base_params, 0);
}
OBJ_DESTRUCT(&mca_base_params);
@ -237,11 +251,12 @@ int mca_base_param_finalize(void)
/*************************************************************************/
static int param_register(const char *type_name, const char *module_name,
static int param_register(const char *type_name, const char *component_name,
const char *param_name, const char *mca_param_name,
mca_base_param_type_t type,
mca_base_param_storage_t *default_value)
{
int ret;
size_t i, len;
mca_base_param_t param, *array;
@ -253,31 +268,38 @@ static int param_register(const char *type_name, const char *module_name,
initialized = true;
}
/* Error check */
if (NULL == type_name) {
return OMPI_ERR_BAD_PARAM;
}
/* Create a parameter entry. If a keyval is to be used, it will be
registered elsewhere. We simply assign -1 here. */
OBJ_CONSTRUCT(&param, mca_base_param_t);
param.mbp_type = type;
param.mbp_keyval = -1;
param.mbp_keyval = MPI_KEYVAL_INVALID;
param.mbp_type_name = strdup(type_name);
if (NULL == param.mbp_type_name) {
return OMPI_ERROR;
OBJ_DESTRUCT(&param);
return OMPI_ERR_OUT_OF_RESOURCE;
}
if (NULL != module_name) {
param.mbp_module_name = strdup(module_name);
if (NULL == param.mbp_module_name) {
free(param.mbp_type_name);
return OMPI_ERROR;
if (NULL != component_name) {
param.mbp_component_name = strdup(component_name);
if (NULL == param.mbp_component_name) {
OBJ_DESTRUCT(&param);
return OMPI_ERR_OUT_OF_RESOURCE;
}
} else {
param.mbp_module_name = NULL;
param.mbp_param_name = NULL;
}
if (param_name != NULL) {
if (NULL != param_name) {
param.mbp_param_name = strdup(param_name);
if (NULL == param.mbp_param_name) {
free(param.mbp_type_name);
free(param.mbp_module_name);
return OMPI_ERROR;
OBJ_DESTRUCT(&param);
return OMPI_ERR_OUT_OF_RESOURCE;
}
} else {
param.mbp_param_name = NULL;
@ -285,17 +307,20 @@ static int param_register(const char *type_name, const char *module_name,
/* The full parameter name may have been specified by the caller.
If it was, use that (only for backwards compatability).
Otherwise, derive it from the type, module, and parameter
Otherwise, derive it from the type, component, and parameter
name. */
param.mbp_env_var_name = NULL;
if (MCA_BASE_PARAM_INFO != mca_param_name && NULL != mca_param_name) {
if (NULL != mca_param_name) {
param.mbp_full_name = strdup(mca_param_name);
if (NULL == param.mbp_full_name) {
OBJ_DESTRUCT(&param);
return OMPI_ERROR;
}
} else {
len = 16 + strlen(type_name);
if (NULL != module_name) {
len += strlen(module_name);
if (NULL != component_name) {
len += strlen(component_name);
}
if (NULL != param_name) {
len += strlen(param_name);
@ -303,22 +328,16 @@ static int param_register(const char *type_name, const char *module_name,
param.mbp_full_name = malloc(len);
if (NULL == param.mbp_full_name) {
if (NULL != param.mbp_type_name) {
free(param.mbp_type_name);
}
if (NULL != param.mbp_module_name) {
free(param.mbp_module_name);
}
if (NULL != param.mbp_param_name) {
free(param.mbp_param_name);
}
OBJ_DESTRUCT(&param);
return OMPI_ERROR;
}
strncpy(param.mbp_full_name, type_name, len);
if (NULL != module_name) {
/* Copy the name over in parts */
strncpy(param.mbp_full_name, type_name, len);
if (NULL != component_name) {
strcat(param.mbp_full_name, "_");
strcat(param.mbp_full_name, module_name);
strcat(param.mbp_full_name, component_name);
}
if (NULL != param_name) {
strcat(param.mbp_full_name, "_");
@ -326,25 +345,19 @@ static int param_register(const char *type_name, const char *module_name,
}
}
/* If mca_param_name isn't MCA_BASE_PARAM_INFO, then it's a
lookup-able value. So alloc the environment variable name as
well. */
/* Create the environment name */
if (MCA_BASE_PARAM_INFO != mca_param_name) {
len = strlen(param.mbp_full_name) + strlen(mca_prefix) + 16;
param.mbp_env_var_name = malloc(len);
if (NULL == param.mbp_env_var_name) {
free(param.mbp_full_name);
free(param.mbp_type_name);
free(param.mbp_module_name);
free(param.mbp_param_name);
return OMPI_ERROR;
}
snprintf(param.mbp_env_var_name, len, "%s%s", mca_prefix,
param.mbp_full_name);
len = strlen(param.mbp_full_name) + strlen(mca_prefix) + 16;
param.mbp_env_var_name = malloc(len);
if (NULL == param.mbp_env_var_name) {
OBJ_DESTRUCT(&param);
return OMPI_ERROR;
}
snprintf(param.mbp_env_var_name, len, "%s%s", mca_prefix,
param.mbp_full_name);
/* Figure out the default value */
/* Figure out the default value; zero it out if a default is not
provided */
if (NULL != default_value) {
if (MCA_BASE_PARAM_TYPE_STRING == param.mbp_type &&
@ -357,49 +370,59 @@ static int param_register(const char *type_name, const char *module_name,
memset(&param.mbp_default_value, 0, sizeof(param.mbp_default_value));
}
/* See if this entry is already in the Array */
/* See if this entry is already in the array */
len = ompi_value_array_get_size(&mca_base_params);
array = OMPI_VALUE_ARRAY_GET_BASE(&mca_base_params, mca_base_param_t);
for (i = 0; i < len; ++i) {
if (0 == strcmp(param.mbp_full_name, array[i].mbp_full_name)) {
/* Copy in the new default value to the old entry */
/* We found an entry with the same param name. Free the old
value (if it was a string */
if (MCA_BASE_PARAM_TYPE_STRING == array[i].mbp_type &&
NULL != array[i].mbp_default_value.stringval) {
free(array[i].mbp_default_value.stringval);
}
if (MCA_BASE_PARAM_TYPE_STRING == param.mbp_type &&
NULL != param.mbp_default_value.stringval) {
array[i].mbp_default_value.stringval =
strdup(param.mbp_default_value.stringval);
/* Now put in the new value */
if (MCA_BASE_PARAM_TYPE_STRING == param.mbp_type) {
if (NULL != param.mbp_default_value.stringval) {
array[i].mbp_default_value.stringval =
strdup(param.mbp_default_value.stringval);
} else {
array[i].mbp_default_value.stringval = NULL;
}
} else {
array[i].mbp_default_value.intval =
param.mbp_default_value.intval;
}
param_free(&param);
/* Just in case we changed type */
array[i].mbp_type = param.mbp_type;
/* Now delete the newly-created entry (since we just saved the
value in the old entry) */
OBJ_DESTRUCT(&param);
return i;
}
}
/* Add it to the array */
if (OMPI_SUCCESS != ompi_value_array_append_item(&mca_base_params, &param)) {
return OMPI_ERROR;
if (OMPI_SUCCESS !=
(ret = ompi_value_array_append_item(&mca_base_params, &param))) {
return ret;
}
return ompi_value_array_get_size(&mca_base_params) - 1;
}
/*
* DO NOT MODIFY THIS FUNCTION WITHOUT ALSO MODIFYING mca_base_param.c!
*
* This function appears in libompi. Because of unix linker semantics,
* it's simply easier to essentially duplicate this function in libmpi
* because in libmpi, we need to lookup on a keyval before looking in
* the environment. The logic is simpler if we just duplicate/alter
* the code in mca_base_param.c rather than try to make this a) public,
* and b) more general (to accomodate looking up keyvals while not
* linking to MPI_Comm_get_attr() in libmpi).
* Lookup a parameter
*/
static bool param_lookup(int index, mca_base_param_storage_t *storage,
ompi_hash_table_t *attrs)
@ -439,13 +462,13 @@ static bool param_lookup(int index, mca_base_param_storage_t *storage,
&storage->stringval, &flag);
if (OMPI_SUCCESS == err && 1 == flag) {
/* Because of alignment weirdness between (void*) and int, it's
simpler to just call ompi_attr_get with the right storage
vehicle vs. trying to cast (extra) to (storage) */
/* Because of alignment weirdness between (void*) and int, we
must grab the lower sizeof(int) bytes from the (char*) in
stringval, in case sizeof(int) != sizeof(char*). */
if (MCA_BASE_PARAM_TYPE_INT == array[index].mbp_type) {
err = ompi_attr_get(attrs, array[index].mbp_keyval,
&storage->intval, &flag);
storage->intval = *((int *) (storage->stringval +
sizeof(void *) - sizeof(int)));
}
/* Nothing to do for string -- we already have the value loaded
@ -494,13 +517,34 @@ static bool param_lookup(int index, mca_base_param_storage_t *storage,
}
static void param_free(mca_base_param_t *p)
/*
* Create an empty param container
*/
static void param_constructor(mca_base_param_t *p)
{
p->mbp_type = MCA_BASE_PARAM_TYPE_MAX;
p->mbp_type_name = NULL;
p->mbp_component_name = NULL;
p->mbp_param_name = NULL;
p->mbp_full_name = NULL;
p->mbp_keyval = -1;
p->mbp_env_var_name = NULL;
p->mbp_default_value.stringval = NULL;
}
/*
* Free all the contents of a param container
*/
static void param_destructor(mca_base_param_t *p)
{
if (NULL != p->mbp_type_name) {
free(p->mbp_type_name);
}
if (NULL != p->mbp_module_name) {
free(p->mbp_module_name);
if (NULL != p->mbp_component_name) {
free(p->mbp_component_name);
}
if (NULL != p->mbp_param_name) {
free(p->mbp_param_name);

Просмотреть файл

@ -19,6 +19,11 @@
* -# Lookup a "normal" parameter value on a specific index, or
* -# Lookup an attribute parameter on a specific index and
* communicator / datatype / window.
*
* Note that there is a second header file (mca_base_param_internal.h)
* that contains several internal type delcarations for the parameter
* system. The internal file is only used within the parameter system
* itself; it should not be required by any other Open MPI entities.
*/
#ifndef OMPI_MCA_BASE_PARAM_H
@ -28,76 +33,6 @@
#include "class/ompi_hash_table.h"
/**
* \internal
*
* Types for MCA parameters.
*/
typedef union {
int intval;
/**< Integer value */
char *stringval;
/**< String value */
} mca_base_param_storage_t;
/** \internal
*
* Special name used to indicate that this is an "info" value.
*/
#define MCA_BASE_PARAM_INFO ((void*) -1)
/**
* \internal
*
* The following types are really in this public .h file so that
* ompi_info can see them. No one else should use them!
*/
typedef enum {
MCA_BASE_PARAM_TYPE_INT,
/**< The parameter is of type integer. */
MCA_BASE_PARAM_TYPE_STRING,
/**< The parameter is of type string. */
MCA_BASE_PARAM_TYPE_MAX
/**< Maximum parameter type. */
} mca_base_param_type_t;
/**
* \internal
*
* Entry for holding the information about an MCA parameter and its
* default value.
*/
struct mca_base_param_t {
mca_base_param_type_t mbp_type;
/**< Enum indicating the type of the parameter (integer or string) */
char *mbp_type_name;
/**< String of the type name, or NULL */
char *mbp_module_name;
/**< String of the component name */
char *mbp_param_name;
/**< String of the parameter name */
char *mbp_full_name;
/**< Full parameter name, in case it is not
<type>_<component>_<param> */
int mbp_keyval;
/**< Keyval value for MPI attribute parameters */
char *mbp_env_var_name;
/**< Environment variable name */
mca_base_param_storage_t mbp_default_value;
/**< Default value of the parameter */
};
/**
* \internal
*
* Convenience typedef.
*/
typedef struct mca_base_param_t mca_base_param_t;
/*
* Global functions for MCA
*/
@ -109,12 +44,11 @@ extern "C" {
* Register an integer MCA parameter.
*
* @param type_name[in] The MCA type (string).
* @param module_name[in] The name of the module (string).
* @param component_name[in] The name of the component (string).
* @param param_name[in] The name of the parameter being registered
* (string).
* @param mca_param_name[in] If NULL, the user-visible name of the
* parameter is {type_name}_{module_name}_{param_name}. If this
* parameter is non-NULL, it is used instead of the default name.
* @param mca_param_name[in] Optional parameter to override the
* user-visible name of this parameter (string).
* @param default_value[in] The value that is used for this
* parameter if the user does not supply one.
*
@ -123,13 +57,33 @@ extern "C" {
* mca_base_param_lookup_int() to retrieve the value of the parameter.
*
* This function registers an integer MCA parameter and associates it
* with a specific module.
* with a specific component.
*
* In most cases, mca_param_name should be NULL. Only in rare cases
* is it necessary (or advisable) to override the default name.
* The default resulting MCA parameter name is
* {type_name}[_{component_name}][_{param_name}].
*
* {component_name} is only included if it is non-NULL. All
* components an should include their name; component frameworks
* should pass "base". It is only permissible for the MCA base
* itself to pass NULL for the component_name.
*
* Likewise, {param_name} is also only included if it is non-NULL.
* Components and frameworks can pass NULL for this parameter if
* they wish.
*
* In most cases, mca_param_name should be NULL, in which case the
* user-visible name of this parameter will be the default form (as
* described above). Only in rare cases is it necessary (or
* advisable) to override the default name -- its use is strongly
* discouraged.
*
* It is permissable to register a (type_name, component_name,
* param_name) triple more than once; the same index value will be
* returned, but the default value will be changed to reflect the
* last registration.
*/
int mca_base_param_register_int(const char *type_name,
const char *module_name,
const char *component_name,
const char *param_name,
const char *mca_param_name,
int default_value);
@ -138,12 +92,11 @@ extern "C" {
* Register a string MCA parameter.
*
* @param type_name[in] The MCA type (string).
* @param module_name[in] The name of the module (string).
* @param component_name[in] The name of the component (string).
* @param param_name[in] The name of the parameter being registered
* (string).
* @param mca_param_name[in] If NULL, the user-visible name of the
* parameter is {type_name}_{module_name}_{param_name}. If this
* parameter is non-NULL, it is used instead of the default name.
* @param mca_param_name[in] Optional parameter to override the
* user-visible name of this parameter (string).
* @param default_value[in] The value that is used for this
* parameter if the user does not supply one.
*
@ -152,14 +105,13 @@ extern "C" {
* mca_base_param_lookup_string() to retrieve the value of the
* parameter.
*
* This function registers an string MCA parameter and associates it
* with a specific module.
*
* In most cases, mca_param_name should be NULL. Only in rare cases
* is it necessary (or advisable) to override the default name.
* This function is identical to mca_base_param_register_int()
* except that you are registering a string parameter with an
* associated string default value (which is allowed to be NULL).
* See mca_base_param_register_int() for all other details.
*/
int mca_base_param_register_string(const char *type_name,
const char *module_name,
const char *component_name,
const char *param_name,
const char *mca_param_name,
const char *default_value);
@ -275,7 +227,7 @@ extern "C" {
* Find the index for an MCA parameter based on its names.
*
* @param type_name Name of the type containing the parameter.
* @param module_name Name of the module containing the parameter.
* @param component_name Name of the component containing the parameter.
* @param param_name Name of the parameter.
*
* @retval OMPI_ERROR If the parameter was not found.
@ -283,14 +235,14 @@ extern "C" {
*
* It is not always convenient to widely propagate a parameter's index
* value, or it may be necessary to look up the parameter from a
* different module -- where it is not possible to have the return
* different component -- where it is not possible to have the return
* value from mca_base_param_register_int() or
* mca_base_param_register_string(). This function can be used to
* look up the index of any registered parameter. The returned index
* can be used with mca_base_param_lookup_int() and
* mca_base_param_lookup_string().
*/
int mca_base_param_find(const char *type, const char *module,
int mca_base_param_find(const char *type, const char *component,
const char *param);
/**

83
src/mca/base/mca_base_param_internal.h Обычный файл
Просмотреть файл

@ -0,0 +1,83 @@
/*
* $HEADER$
*/
/** @file This is the private declarations for the MCA parameter
* system. This file is internal to the MCA parameter system and
* should not need to be used by any other elements in Open MPI except
* the special case of the ompi_info command.
*/
#ifndef OMPI_MCA_BASE_PARAM_INTERNAL_H
#define OMPI_MCA_BASE_PARAM_INTERNAL_H
#include "mpi.h"
#include "class/ompi_object.h"
#include "class/ompi_hash_table.h"
/**
* Types for MCA parameters.
*/
typedef union {
int intval;
/**< Integer value */
char *stringval;
/**< String value */
} mca_base_param_storage_t;
/**
* The following types are really in this public .h file so that
* ompi_info can see them. No one else should use them!
*/
typedef enum {
MCA_BASE_PARAM_TYPE_INT,
/**< The parameter is of type integer. */
MCA_BASE_PARAM_TYPE_STRING,
/**< The parameter is of type string. */
MCA_BASE_PARAM_TYPE_MAX
/**< Maximum parameter type. */
} mca_base_param_type_t;
/**
* Entry for holding the information about an MCA parameter and its
* default value.
*/
struct mca_base_param_t {
ompi_object_t mbp_super;
/**< Allow this to be an OMPI OBJ */
mca_base_param_type_t mbp_type;
/**< Enum indicating the type of the parameter (integer or string) */
char *mbp_type_name;
/**< String of the type name, or NULL */
char *mbp_component_name;
/**< String of the component name */
char *mbp_param_name;
/**< String of the parameter name */
char *mbp_full_name;
/**< Full parameter name, in case it is not
<type>_<component>_<param> */
int mbp_keyval;
/**< Keyval value for MPI attribute parameters */
char *mbp_env_var_name;
/**< Environment variable name */
mca_base_param_storage_t mbp_default_value;
/**< Default value of the parameter */
};
/**
* Convenience typedef.
*/
typedef struct mca_base_param_t mca_base_param_t;
/**
* Object delcataion for mca_base_param_t
*/
OBJ_CLASS_DECLARATION(mca_base_param_t);
#endif /* OMPI_MCA_BASE_PARAM_INTERNAL_H */