Hi, It would be good to not make public more than what is necessary. As long as it keep compiling, there does not seem to be any harm. Unless Ceph considers the crush library API to be published and maintained ? I mean that it could require re-compilation or even refactor of third party code using the crush library independantly for tools external to Ceph. Cheers On 04/09/2014 10:53, Chen, Xiaoxi wrote: > Hi List, > The CrushWrapper(https://github.com/ceph/ceph/blob/master/src/crush/CrushWrapper.cc) is a wrapper for crush so we can use such C++ wrapper instead of playing directly with crush C-API. > But currently, in CrushWrapper, the member struct crush_map *crush is a public member, so people can break the encapsulation and manipulate directly to the crush structure. I think this is not a good practice for encapsulation and will lead to inconsistent if code mix use the CrushWrapper API and crush C API, a simple example could be : if you use crush_add_rule(C-API) to add a rule, which will not set the have_rmap flag to false in CrushWrapper, then other code using CrushWrapper trying to look up the newly added rule by name will get a -ENOENT. > So I am not sure why this member is public currently? And what's the impact if we wish to clean up and make it private? Any hard-to-fix dependency? > > Thanks > Xiaoxi > -- Loïc Dachary, Artisan Logiciel Libre
Attachment:
signature.asc
Description: OpenPGP digital signature
