search

linux-rdma / opensm

Other
66 stars 36 forks source link

libosmcomp.so calls exit #8

Closed bdrung closed 5 years ago

bdrung commented 5 years ago

The shared library libosmcomp.so calls the C library exit() or _exit() functions.

In the case of an error, the library should instead return an appropriate error code to the calling program which can then determine how to handle the error, including performing any required clean-up.

hnrose commented 5 years ago

(I think that) the only occurrence is in complib_init function which returns void ;-(

complin_init function API could be changed to return status rather than void and fix internal uses of this function to exit on bad status but that breaks out of tree uses.

A new function (complib_init_v2) that returns status could easily be added and change all internal uses to that as well as deprecate the old function but old function wouldn't be removed for several releases.

To me latter approach is least bad or do you see other/better approach ?

bdrung commented 5 years ago

Besides complib_init, __hca_sim_get_num_cas calls exit() which is used by osm_vendor_get_all_port_attr, osm_vendor_get_guid_ca_and_port, and osm_vendor_get_guid_by_ca_and_port.

The latter approach sound better to me as well.

hnrose commented 5 years ago

Implemented patch to deprecate complib_init and add complib_init_v2. Sent to you for review.

The other routines you mentioned are in libvendor. I will work on those tomorrow in a separate patch.

hnrose commented 5 years ago

Fixes are pretty simple for libvendor/osm_vendor_mlx_hca_sim.c and libvendor/osm_vendor_mlx_sim.c but not sure if it's worth doing this as ibutils (which contains ibmgtsim) has been deprecated for quite some time.

hnrose commented 5 years ago

Just sent patch with the libvendor changes.

bdrung commented 5 years ago

Reviewed and test-build both patches. They look good to me.