[Gluster-devel] problem with recent change to glfs_realpath

[Niels de Vos]

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 :-/

> 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().

> 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.

I was (and still am!) planning to write a blog post and email about this
general glfs_free() addition/change.

Sorry for the inconvenience,
Niels
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20161021/8d4c3dee/attachment.sig>