linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Ben Hutchings <ben@decadent.org.uk>
Cc: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>,
	Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
	Linux Trace Devel <linux-trace-devel@vger.kernel.org>
Subject: Re: tep_free_plugin_paths() function in libtraceevent
Date: Tue, 15 Sep 2020 09:41:40 -0400	[thread overview]
Message-ID: <20200915094140.25474a64@gandalf.local.home> (raw)
In-Reply-To: <466701766794299d1c238f450c89fac07e43744b.camel@decadent.org.uk>

On Tue, 15 Sep 2020 14:26:48 +0100
Ben Hutchings <ben@decadent.org.uk> wrote:

> On Tue, 2020-09-15 at 16:12 +0300, Tzvetomir Stoyanov wrote:
> > On Tue, Sep 15, 2020 at 12:29 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:  
> > > Em Fri, Sep 11, 2020 at 03:08:16AM +0100, Ben Hutchings escreveu:  
> > > > I noticed that the new function tep_free_plugin_paths() is exported
> > > > from libtraceevent, but is only declared in a private header file.  
> > 
> > Hi Ben,
> > The tep_free_plugin_paths() function is supposed to be an internal
> > function, not an API - that's why it is in a private header. What do you
> > mean by "exported": the "tep_" prefix, or the fact that it is not static?  
> 
> It is exported from the shared library, i.e. it's included in the
> library's dynamic symbols.  That means that it could be called by a
> program using the library, and would conflict with a function defined
> by the program.

Of course a program that uses this shouldn't have any tep_ prefix
functions ;-)

> 
> > I can remove the prefix, if that is what bothers you.  
> 
> No, exporting symbols without that prefix would make conflicts more
> likely.

Right!

> 
> > The function is
> > defined in event-plugin.c and is used in event-parse.c, that's why it
> > cannot be made static without a library redesign.  
> 
> Yes, I realise that.  However you can define it as "hidden":
> <https://gcc.gnu.org/onlinedocs/gcc-10.2.0/gcc/Common-Function-Attributes.html#index-visibility-function-attribute>.

Ah, that's how libraries can hide non static functions. I learned something
new today!

Tzvetomir,


Let's only have functions that are part of the library's API start with
"tep_", and all other functions not have "tep_" in the name (so we know
which ones are exported and which ones are not).

Then any function that doesn't start with "tep_" should either be static,
or hidden.

#define __hidden __attribute__((visibility ("hidden")))

static void foo ()
{
}

void __hidden bar ()
{
}


-- Steve


      reply	other threads:[~2020-09-16  0:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-11  2:08 tep_free_plugin_paths() function in libtraceevent Ben Hutchings
2020-09-14 21:29 ` Arnaldo Carvalho de Melo
2020-09-15 13:12   ` Tzvetomir Stoyanov
2020-09-15 13:26     ` Ben Hutchings
2020-09-15 13:41       ` Steven Rostedt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200915094140.25474a64@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=arnaldo.melo@gmail.com \
    --cc=ben@decadent.org.uk \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=tz.stoyanov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).