Re: [PATCH v2 3/9] kernel-shark: Fix KS_DEFINE_PLUGIN_CONTEXT macro

[Yordan Karadzhov <y.karadz@gmail.com>]

On 6.05.21 г. 21:11, Steven Rostedt wrote:
> On Wed, 28 Apr 2021 16:47:24 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> The KS_DEFINE_PLUGIN_CONTEXT macro implements methods that are used
>> to deal with plugin-specific context objects. However, when this macro
>> is used in multiple plugins and those plugins are loaded together
>> the symbol resolving fails, resulting in undefined behavior. Namely,
>> version of the function from one plugin, being called by another
>> plugin. Here we make sure that the methods defined in
>> KS_DEFINE_PLUGIN_CONTEXT are not visible outside of the corresponding
>> plugin.
>>
>> Fixing: 15df009 (kernel-shark: Add KS_DEFINE_PLUGIN_CONTEXT macro)
>> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>> ---
>>   src/libkshark-plugin.h     | 22 ++++++++++++++++++----
>>   src/plugins/sched_events.c |  3 +++
>>   src/plugins/sched_events.h |  3 +--
>>   3 files changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/libkshark-plugin.h b/src/libkshark-plugin.h
>> index c110616..752dbeb 100644
>> --- a/src/libkshark-plugin.h
>> +++ b/src/libkshark-plugin.h
>> @@ -24,6 +24,8 @@ extern "C" {
>>   /* Quiet warnings over documenting simple structures */
>>   //! @cond Doxygen_Suppress
>>   
>> +#define __hidden __attribute__((visibility ("hidden")))
>> +
>>   #define _MAKE_STR(x)	#x
>>   
>>   #define MAKE_STR(x)	_MAKE_STR(x)
>> @@ -364,11 +366,14 @@ int kshark_handle_all_dpis(struct kshark_data_stream *stream,
>>   	__ok;								\
>>   })									\
>>   
>> -/** General purpose macro defining methods for adding plugin context. */
>> +/**
>> + * General purpose macro defining methods for adding plugin context.
>> + * Do not use this macro in header files.
>> + */
>>   #define KS_DEFINE_PLUGIN_CONTEXT(type)					\
>>   static type **__context_handler;					\
>>   static ssize_t __n_streams = -1;					\
>> -static inline type *__init(int sd)					\
>> +__hidden type *__init(int sd)						\
> 
> This is strange, because static inline should never be a problem in this
> regard (otherwise things will break elsewhere too).
> 
> static is never exported (it's stronger than "hidden").
> 
> Can you show me how you see this error, because this solution does not make
> any sense.

The problem is that some plugins can build from multiple source files. 
For example in the case when part of the plugin is written in C and 
another part in C++. In those cases we cannot have the functions being 
static.

Thanks!
Yordan


> 
> -- Steve
>