[Gluster-devel] Mark all the xlator fops 'static '

[Kaleb S. KEITHLEY]

On 07/31/2015 02:03 AM, Rajesh Joseph wrote:
>> <snip>
>> Adding the 'static' keyword to functions keeps their scope inside the
>> compiled object code (.o files) and references are not dynamically
>> loaded through the dynamic linker. Because the dynamic linker is not
>> involved in the 'static' functions, the problem of conflicting symbol
>> names does not occur anymore.


A) Making all the fop fns static _works_ (sort of) for snapview-client,
because the fops table and the fop fns are all in the same file.

It does _not_ work for, e.g., dht, where many or all of the fop methods
are in other files than the 'main' dht.c file with the fops (and other)
tables.


B) In his email Shyam pointed to gnu linker documentation about the use
of 'hidden' and 'exported' attributes that can be used to limit the
scope or visibility of symbols in the shared lib. This is almost
certainly not portable to Mac/Darwin.


C) Another possiblity is to link with --version-script (but not actually
use any symbol versions). E.g. a version script for most xlators might
look like this:

{
	global:
		init;
		fini;
		fops;
		cbks;
		options;
		notify;
		mem_acct_init;
		reconfigure;
	local: *;
};

Using `libtool -export-symbols $symbol-file` does this under the covers
(on linux) and is theoretically portable to Mac/Darwin.


D) Still another option might be to link xlators with -Bsymbolic.
    ...
    -Bsymbolic
    When creating a shared library, bind references to global
    symbols to the definition within the shared library, if
    any. Normally, it is possible for a program linked against
    a shared library to override the definition within the
    shared library. This option is only meaningful on ELF
    platforms which support shared libraries.
    ...

If you compile to assembly you'll see that fop fns are global:
    ...
    .globl svc_lookup
    .type svc_lookup, @function
    ...

They are not global when defined static. As with using static, linking
with -Bsymbolic theoretically should work, but would require more
testing to be confident that it works in all cases.


E) Finally, using namespace, i.e. using gf_ prefixes. Requires touching
a lot of files.


Finally, a gratuitous friendly reminder to stop using _ and __ prefixes
for internal/private functions. _ and __ are reserved by the C language.
And also correct, appropriate use of static where it's warranted.



>>
>> Functions that are called from other components (library-like functions)
>> can not have the 'static' keyword. If there is a conflict between
>> function names provided in an OS library and a Gluster component, we
>> should prepend the function name with "gf_" like we have done in the
>> uuid case mentioned below.
>>
> 
> I think as a general guideline all those functions which does not require 
> external linkage should be made static. And those with external linkage 
> should have some unique prefix. e.g. as Niels mentioned "gf_".
> 
> Regards,
> Rajesh
> 
>> Cheers,
>> Niels
>>
>>>
>>> Similar issue with respect to uuid* routines was already reported in [3].
>>>
>>> To avoid such issues, it may be better and a cleaner way to mark all the
>>> xlator fops static, like in [4].
>>>
>>> We have raised bug1248669 to track all the required changes. Request the
>>> component maintainers to take a look and change the respective xlator fop
>>> definitions.
>>>
>>> Thanks,
>>> Soumya
>>>
>>> [1] - https://bugzilla.redhat.com/show_bug.cgi?id=1245636#c6
>>> [2] - https://bugzilla.redhat.com/show_bug.cgi?id=1248669
>>> [3] - http://article.gmane.org/gmane.comp.file-systems.gluster.devel/10074
>>> [4] - http://review.gluster.org/#/c/11805
>> _______________________________________________
>> Gluster-devel mailing list
>> Gluster-devel at gluster.org
>> http://www.gluster.org/mailman/listinfo/gluster-devel
>>
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at gluster.org
> http://www.gluster.org/mailman/listinfo/gluster-devel
> 

-- 

Kaleb