* tep_free_plugin_paths() function in libtraceevent
@ 2020-09-11 2:08 Ben Hutchings
2020-09-14 21:29 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2020-09-11 2:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-trace-devel
[-- Attachment #1: Type: text/plain, Size: 566 bytes --]
I noticed that the new function tep_free_plugin_paths() is exported
from libtraceevent, but is only declared in a private header file.
If it's meant to be part of the API, it should be declared in a public
header file. If not, I think it should be hidden from library users.
(I think there are other only functions with this issue; this just came
to my attention because the Debian packaging tools prompted me to
update the symbol-to-minimum-version mapping.)
Ben.
--
Ben Hutchings
Never put off till tomorrow what you can avoid all together.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: tep_free_plugin_paths() function in libtraceevent
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
0 siblings, 1 reply; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-09-14 21:29 UTC (permalink / raw)
To: Tzvetomir Stoyanov; +Cc: Steven Rostedt, Ben Hutchings, linux-trace-devel
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.
>
> If it's meant to be part of the API, it should be declared in a public
> header file. If not, I think it should be hidden from library users.
>
> (I think there are other only functions with this issue; this just came
> to my attention because the Debian packaging tools prompted me to
> update the symbol-to-minimum-version mapping.)
Tzvetomir, can you please take a look?
- Arnaldo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: tep_free_plugin_paths() function in libtraceevent
2020-09-14 21:29 ` Arnaldo Carvalho de Melo
@ 2020-09-15 13:12 ` Tzvetomir Stoyanov
2020-09-15 13:26 ` Ben Hutchings
0 siblings, 1 reply; 5+ messages in thread
From: Tzvetomir Stoyanov @ 2020-09-15 13:12 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Arnaldo Carvalho de Melo, Steven Rostedt, Linux Trace Devel
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?
I can remove the prefix, if that is what bothers you. 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.
> >
> > If it's meant to be part of the API, it should be declared in a public
> > header file. If not, I think it should be hidden from library users.
> >
> > (I think there are other only functions with this issue; this just came
> > to my attention because the Debian packaging tools prompted me to
> > update the symbol-to-minimum-version mapping.)
>
> Tzvetomir, can you please take a look?
>
> - Arnaldo
--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: tep_free_plugin_paths() function in libtraceevent
2020-09-15 13:12 ` Tzvetomir Stoyanov
@ 2020-09-15 13:26 ` Ben Hutchings
2020-09-15 13:41 ` Steven Rostedt
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2020-09-15 13:26 UTC (permalink / raw)
To: Tzvetomir Stoyanov
Cc: Arnaldo Carvalho de Melo, Steven Rostedt, Linux Trace Devel
[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]
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.
> I can remove the prefix, if that is what bothers you.
No, exporting symbols without that prefix would make conflicts more
likely.
> 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>.
Ben.
> > > If it's meant to be part of the API, it should be declared in a public
> > > header file. If not, I think it should be hidden from library users.
> > >
> > > (I think there are other only functions with this issue; this just came
> > > to my attention because the Debian packaging tools prompted me to
> > > update the symbol-to-minimum-version mapping.)
> >
> > Tzvetomir, can you please take a look?
> >
> > - Arnaldo
>
>
--
Ben Hutchings
I'm not a reverse psychological virus.
Please don't copy me into your signature.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: tep_free_plugin_paths() function in libtraceevent
2020-09-15 13:26 ` Ben Hutchings
@ 2020-09-15 13:41 ` Steven Rostedt
0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-09-15 13:41 UTC (permalink / raw)
To: Ben Hutchings
Cc: Tzvetomir Stoyanov, Arnaldo Carvalho de Melo, Linux Trace Devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-16 0:35 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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).