Re: [PATCH] trace-cmd: libtracecmd: Rename private functions to fix static building

On 31/10/2024 12:47 am, Steven Rostedt wrote:
> On Mon, 28 Oct 2024 12:22:47 +0000
> Metin Kaya <[email protected]> wrote:
> 
>> Building trace-cmd statically fails because of duplicated symbols for
>> strstrip() function which is also implemented in libtracefs. The
>> __hidden attribute does not resolve these conflicts for static builds
>> due to the lack of namespacing support in C. Refer to the associated
>> Bugzilla page [1] for further details.
>>
>> Although only strstrip() breaks the static build as of now, we should
>> fix the underlying issue comprehensively across the libtraceevent,
>> libtracefs, and trace-cmd packages. The recommendation on [1] is
>> prepending package name (e.g., "tracecmd_") to private functions (tagged
>> with __hidden attribute).
>>
>> Thus:
>> 1. Retain private functions which already start with "tracecmd_".
>> 2. If a private function starts with "trace_", then just change its
>>     prefix to "tracecmd_".
>> 3. If prepending "tracecmd_" prefix to a private function's name clashes
>>     with an existing one, rename one of the functions per its context
>>     (e.g., rename trace_load_plugins() to
>>     tracecmd_load_plugins_from_handle() and trace_append_options() to
>>     tracecmd_append_options_to_file()).
>> 4. Prepend "tracecmd_" prefix to all remaining "__hidden" functions.
>>
>> Future __hidden functions should follow this prefixing schemed to avoid
>> new naming conflicts.
> 
> I'm not against the change to make the hidden functions with a unique
> name, but I rather come up with something other than "tracecmd_". That
> prefix was to be used for functions that will eventually become public.
> 
> Would "tcmd_" work?
> 
> Same goes for the other libraries. The "tracefs_" is for functions that
> should be exported. Perhaps rename it to "tfs_"?

Hi Steve,

Ah, you are right. Those prefixes conflict with public ones. Perhaps I 
misread the discussion on Bugzilla page. I've no objection to using 
"tcmd_", "tfs_" and "tep_" prefixes. Will update all three patches 
accordingly.

> 
> As for libtraceevent, it doesn't need a separate prefix for local
> variables compared to exported ones, but that prefix is "tep_" and not
> "traceevent_".

I believe I did not rename any variables in the libtraceevent patch [1].
However, I made the following changes in the libtracefs patch [2]:

-extern const struct tep_format_field common_stacktrace;
+extern const struct tep_format_field tracefs_common_stacktrace;

-extern pthread_mutex_t toplevel_lock;
+extern pthread_mutex_t tracefs_toplevel_lock;

Do you recommend keeping them as is or using "tfs_" prefix?

[1] 
https://lore.kernel.org/linux-trace-devel/[email protected]
[2] 
https://lore.kernel.org/linux-trace-devel/[email protected]

Thanks,