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 > <mca_base_param_deregister_fix.patch>_______________________________________________ > devel mailing list > [email protected] > http://www.open-mpi.org/mailman/listinfo.cgi/devel -- Jeff Squyres [email protected] For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/
