On 2016-10-21 at 10:22 +0200, Niels de Vos wrote: > On Fri, Oct 21, 2016 at 01:15:50AM +0200, Michael Adam wrote: > > Hi all, > > > > Anoop has brought to my attention that > > recently glfs_realpath was changed in an incompatible way: > > Interesting, Rajesh and I had an email discussion about that yesterday > too... Unfortunately this (or the Samba) list was not on CC :-/ Yeah, Anoop brought it up, but Anoop, Rajesh and I discussed the problem and the solution. Since I saw now mail on the list I raised it, but it's got that common background. :-) > > Previously, glfs_realpath returned an allocated string > > that the caller would have to free with 'free'. Now after > > the change, free segfaults on the returned string, and > > the caller needs to call glfs_free. > > > > That change makes no sense, imho, because the result from > > a realpath implementation may be used by the application > > using libgfapi, outside of the scope of the actual libgfapi > > using code. E.g. in samba, the gfapi calls are hidden behind > > the vfs api in the gluster backend. But the realpath result > > is used outside the vfs module. I think this should be quite > > normal a use case, and hence glfs_realpath should behave > > as one would expect from a realpath implementation > > (and as described in the realpath(3) manpage): return a string > > that needs to be freed with 'free'... > > With libgfapi and other applications we can not guarantee that the > malloc() that libgfapi uses matches the free() from the application. It > is possible for applications to link with alternative implementations > (libjemalloc for example). All structures allocated and returned by > libgfapi should be free'd by libgfapi as well. We have seen problems > like this with NFS-Ganesha before, and that triggered us to introduce > glfs_free(). Hm. Ok, I see what you're saying here, but doesn't that apply to many other libraries as well? libc functions allocate memory for you frequently. (Eg libc's realpath.) So no application should ever use these? It kind of questions the fundamentals of many such libs. > > Now for samba, after thorough discussion with Anoop and Rajesh, > > we have proposed a fix/workaround by using the variant of > > glfs_realpath that hands in a pre-allocated result string. > > This renders us independent of the allocation method used by > > glfs_realpath. But we wanted to point this out to the list, since > > it is a potential problem for other users, too. > > That is the correct approach. I am very sorry that I missed to check > samba/vfs_gluster for glfs_realpath() usage, none of the other projects > that I am aware of use this function. No damage done, just some amount of initial confusion. It is just in master yet. By early (manual) upstream testing Anoop found it. We should aim to include samba in some upstream automated sanity test at least. > I was (and still am!) planning to write a blog post and email about this > general glfs_free() addition/change. Sure, that might be a good idea! Cheers - Michael
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
