I have not tested this, but the patch looks reasonable to me.
On Nov 19, 2012, at 3:56 PM, Nathan Hjelm wrote:
> What: mca_base_param provides a deregister function. In the trunk this function is only used in a ft path:
> ompi-trunk hjelmn$ find . -name \*.[ch] | xargs grep mca_base_param_deregister
> ./ompi/mca/bml/r2/bml_r2_ft.c: mca_base_param_deregister(param_type);
> Right now this function is broken for two reasons:
> 1) LEAK: The function does not destruct the mca_base_param_t it is deregistering.
> 2) It removes the parameter from the array and moves every parameter after it up by one index. This will break any code which stores and later uses the parameter index! This isn't a big problem now but will
> become one as MPI_T functionality is added to Open MPI.
> The attached patch does the following:
> - modify mca_base_param_deregister to correctly clean up the deleted parameter,
> - modify the mca_base_param_t destructor to set the type to MCA_BASE_PARAM_TYPE_MAX, and
> - added parameter validity checks where they were missing (an invalid parameter has type MCA_BASE_PARAM_TYPE_MAX.
> Why: If MCA is to support deregistering mca parameters (I don't see why it shouldn't) it needs to be done correctly.
> When: This patch is not critical but I would like feedback on it soon. Timeout set for next Monday (Nov. 26 @ 12:00PM MST).
> Background: This is another part of a larger cleanup of the MCA system in preperation for supporting the MPI tool interface specified in MPI-3.0. In this case mca has a function that can change parameter ind
> ices. This is not allowed by the MPI_T standard.
> Please review the patch for correctness, bugs, etc.
> -Nathan Hjelm
> HPC-3, LANL
> devel mailing list
For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/