Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Savolainen, Petri (Nokia - FI/Espoo)]

Anders,

Have you tried to move pragmas out of the spec file?

I think this (plat header file) is the correct place for the pragmas since 
those are part of the build system and build systems are implementation 
specific. ODP API _spec_ does not dictate any build system or compiler, and 
thus should  not contain anything extra for any single build system. A valid 
implementation could implement everything even in a single file 
(odp/include/odp_api.h).


-Petri


From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT 
Savolainen, Petri (Nokia - FI/Espoo)
Sent: Friday, April 15, 2016 2:02 PM
To: EXT Mike Holmes <mike.hol...@linaro.org>; Maxim Uvarov 
<maxim.uva...@linaro.org>
Cc: lng-odp <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

Agree, that it gets ugly (for interface specification readability), if GCC 
pragmas are added into spec header files. Wouldn’t it work the same way if the 
pragmas are defined where the spec file is included. Namely e.g. here for the 
barrier:


#ifndef ODP_PLAT_BARRIER_H_
#define ODP_PLAT_BARRIER_H_

#ifdef __cplusplus
extern "C" {
#endif

#include 
#include 
#include 
#include 

if __GNUC__ >= 4
#pragma GCC visibility push(default)
#endif

#include 

if __GNUC__ >= 4
#pragma GCC visibility pop
#endif


#ifdef __cplusplus
}
#endif

#endif


-Petri


From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Mike 
Holmes
Sent: Friday, April 15, 2016 1:44 PM
To: Maxim Uvarov <maxim.uva...@linaro.org<mailto:maxim.uva...@linaro.org>>
Cc: lng-odp <lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>>
Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

Maxim it needs to be in the spec, it is the spec that defines the ABI, if you 
dont do this then everyone implementing will have to manually mark hundreds of 
other apis as internal rather than we just export the correct list.

At least that is my understanding at least.



On 15 April 2016 at 06:38, Maxim Uvarov 
<maxim.uva...@linaro.org<mailto:maxim.uva...@linaro.org>> wrote:
Petri, please review this patch. For me it's not clear why it touches 
include/odp/api/spec files at all.

Maxim.

On 04/13/16 22:06, Bill Fischofer wrote:
Ok, thanks.  With that:

Reviewed-by: Bill Fischofer 
<bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> 
<mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>>

On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti 
<ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org> 
<mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>> wrote:

On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
<bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> 
<mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>> wrote:
> On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
> <ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org> 
<mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>>
wrote:
>>
>> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
>> <bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> 
<mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>>
wrote:
>> >
>> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
>> > <anders.rox...@linaro.org<mailto:anders.rox...@linaro.org> 
<mailto:anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>>>
>> > wrote:
>> >>
>> >> Internal functions should not be part of symbols that are
visible
>> >> outside the library. Using -fvisibility=hidden hides all
internal
>> >> functions from the public ABI.
>> >>
>> >> Suggested-by: Ricardo Salveti 
<ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>
<mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>>
>> >> Signed-off-by: Anders Roxell 
<anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>
<mailto:anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>>>

>> >> ---
>> >>  include/odp/api/spec/align.h |  8 
>> >>  include/odp/api/spec/atomic.h  |  8 
>> >>  include/odp/api/spec/barrier.h |  8 
>> >>  include/odp/api/spec/buffer.h  |  8 
>> >>  include/odp/api/spec/byteorder.h |  8 
>> >> include/odp/api/spec/classification.h |  8 
>> >>  include/odp/api/spec/compiler.h  |  8 
&

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Bill Fischofer]

The argument for the #pragma in the ODP API files is that by design they
should contain nothing that isn't intended for export, so not marking each
struct/API individually both saves time and discourages someone trying to
put something private in those files (by not marking the individual entity).

It's unfortunate that by design a C macro cannot emit a #pragma as this
means you can't hide the somewhat awkward syntax.  An alternative would be
to wrap these #pragmas in separate #include files., e.g.:

#include 
...odp API file contents
#include 



On Fri, Apr 15, 2016 at 11:33 AM, Mike Holmes 
wrote:

> Thanks all, this is on Mondays ARCH call to expedite things since we want
> to get 1.9 out.
>
> On 15 April 2016 at 11:59, Ricardo Salveti 
> wrote:
>
>> On Fri, Apr 15, 2016 at 12:24 PM, Maxim Uvarov 
>> wrote:
>> > We need more review for that patch.
>> >
>> > From one side it's good changes and logically it's right to make api
>> visible
>> > and other functions not visible by default. From other point of view I
>> can
>> > not find any other libs which do exactly the same thing. So it's not
>> clear
>> > what we can break with that changes, if we can.
>>
>> We do have quite a few other examples of libraries and projects using
>> a similar approach (e.g. mesa, wayland, tslib), but they are usually
>> controlling symbol by symbol instead of using pragma for the entire
>> header.
>>
>> A common example is defining EXPORT as __attribute__
>> ((visibility("default"))) and then manually having that as part of
>> each public symbol/declaration.
>>
>> So I guess the decision to make is if we want to have that as part of
>> the header, and if pragma or manual exposure via __attribute__ should
>> be used.
>>
>> Cheers,
>> --
>> Ricardo Salveti
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org  *│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"
>
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Mike Holmes]

Thanks all, this is on Mondays ARCH call to expedite things since we want
to get 1.9 out.

On 15 April 2016 at 11:59, Ricardo Salveti 
wrote:

> On Fri, Apr 15, 2016 at 12:24 PM, Maxim Uvarov 
> wrote:
> > We need more review for that patch.
> >
> > From one side it's good changes and logically it's right to make api
> visible
> > and other functions not visible by default. From other point of view I
> can
> > not find any other libs which do exactly the same thing. So it's not
> clear
> > what we can break with that changes, if we can.
>
> We do have quite a few other examples of libraries and projects using
> a similar approach (e.g. mesa, wayland, tslib), but they are usually
> controlling symbol by symbol instead of using pragma for the entire
> header.
>
> A common example is defining EXPORT as __attribute__
> ((visibility("default"))) and then manually having that as part of
> each public symbol/declaration.
>
> So I guess the decision to make is if we want to have that as part of
> the header, and if pragma or manual exposure via __attribute__ should
> be used.
>
> Cheers,
> --
> Ricardo Salveti
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Bill Fischofer]

On Fri, Apr 15, 2016 at 6:39 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:

>
>
> > -Original Message-
> > From: EXT Anders Roxell [mailto:anders.rox...@linaro.org]
> > Sent: Friday, April 15, 2016 2:24 PM
> > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com>
> > Cc: EXT Mike Holmes <mike.hol...@linaro.org>; Maxim Uvarov
> > <maxim.uva...@linaro.org>; lng-odp <lng-odp@lists.linaro.org>
> > Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible
> >
> > On 15 April 2016 at 13:02, Savolainen, Petri (Nokia - FI/Espoo)
> > <petri.savolai...@nokia.com> wrote:
> > > Agree, that it gets ugly (for interface specification readability), if
> > GCC
> > > pragmas are added into spec header files. Wouldn’t it work the same way
> > if
> > > the pragmas are defined where the spec file is included. Namely e.g.
> > here
> > > for the barrier:
> >
> > I guess it would work, haven't tried. That means that all the
> > implementations
> > have to add this into their files.
> > If we what we have in this patch implementations gets it for "free".
> >
> > Cheers,
> > Anders
> >
>
> Since all implementations may not support binary compatible libs or even
> GCC. I think it belong to implementation and those may solve it in
> different ways. So, I don't think that "get for free" is as important here
> as a clean set API spec files.
>

These changes enable but do not force ABI mode and they work for both GCC
and clang.  Enabling ABI mode involves compiling with -fvisibility=hidden.
When you do that then all symbols are hidden by default and the result is
only the ODP APIs are exposed, due to these pragmas.

This is exactly what we want.  If an implementation wishes to expose
symbols other than those that are part of the ODP API then it can simply
follow this override model for those symbols.


>
> -Petri
>
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Savolainen, Petri (Nokia - FI/Espoo)]

> -Original Message-
> From: EXT Anders Roxell [mailto:anders.rox...@linaro.org]
> Sent: Friday, April 15, 2016 2:24 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com>
> Cc: EXT Mike Holmes <mike.hol...@linaro.org>; Maxim Uvarov
> <maxim.uva...@linaro.org>; lng-odp <lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible
> 
> On 15 April 2016 at 13:02, Savolainen, Petri (Nokia - FI/Espoo)
> <petri.savolai...@nokia.com> wrote:
> > Agree, that it gets ugly (for interface specification readability), if
> GCC
> > pragmas are added into spec header files. Wouldn’t it work the same way
> if
> > the pragmas are defined where the spec file is included. Namely e.g.
> here
> > for the barrier:
> 
> I guess it would work, haven't tried. That means that all the
> implementations
> have to add this into their files.
> If we what we have in this patch implementations gets it for "free".
> 
> Cheers,
> Anders
> 

Since all implementations may not support binary compatible libs or even GCC. I 
think it belong to implementation and those may solve it in different ways. So, 
I don't think that "get for free" is as important here as a clean set API spec 
files.

-Petri 


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Mike Holmes]

On 15 April 2016 at 07:23, Anders Roxell <anders.rox...@linaro.org> wrote:

> On 15 April 2016 at 13:02, Savolainen, Petri (Nokia - FI/Espoo)
> <petri.savolai...@nokia.com> wrote:
> > Agree, that it gets ugly (for interface specification readability), if
> GCC
> > pragmas are added into spec header files. Wouldn’t it work the same way
> if
> > the pragmas are defined where the spec file is included. Namely e.g. here
> > for the barrier:
>
> I guess it would work, haven't tried. That means that all the
> implementations
> have to add this into their files.
> If we what we have in this patch implementations gets it for "free".
>

I agree with Anders, the spec should define what the API IS, rather than
have all implementations exclude what is not in the API


>
> Cheers,
> Anders
>
> >
> >
> >
> >
> >
> > #ifndef ODP_PLAT_BARRIER_H_
> >
> > #define ODP_PLAT_BARRIER_H_
> >
> >
> >
> > #ifdef __cplusplus
> >
> > extern "C" {
> >
> > #endif
> >
> >
> >
> > #include 
> >
> > #include 
> >
> > #include 
> >
> > #include 
> >
> >
> >
> > if __GNUC__ >= 4
> > #pragma GCC visibility push(default)
> > #endif
> >
> >
> >
> > #include 
> >
> >
> >
> > if __GNUC__ >= 4
> > #pragma GCC visibility pop
> > #endif
> >
> >
> >
> >
> >
> > #ifdef __cplusplus
> >
> > }
> >
> > #endif
> >
> >
> >
> > #endif
> >
> >
> >
> >
> >
> > -Petri
> >
> >
> >
> >
> >
> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
> > Mike Holmes
> > Sent: Friday, April 15, 2016 1:44 PM
> > To: Maxim Uvarov <maxim.uva...@linaro.org>
> > Cc: lng-odp <lng-odp@lists.linaro.org>
> > Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible
> >
> >
> >
> > Maxim it needs to be in the spec, it is the spec that defines the ABI, if
> > you dont do this then everyone implementing will have to manually mark
> > hundreds of other apis as internal rather than we just export the correct
> > list.
> >
> >
> >
> > At least that is my understanding at least.
> >
> >
> >
> >
> >
> >
> >
> > On 15 April 2016 at 06:38, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
> >
> > Petri, please review this patch. For me it's not clear why it touches
> > include/odp/api/spec files at all.
> >
> > Maxim.
> >
> > On 04/13/16 22:06, Bill Fischofer wrote:
> >
> > Ok, thanks.  With that:
> >
> > Reviewed-by: Bill Fischofer <bill.fischo...@linaro.org
> > <mailto:bill.fischo...@linaro.org>>
> >
> > On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti <
> ricardo.salv...@linaro.org
> > <mailto:ricardo.salv...@linaro.org>> wrote:
> >
> > On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
> > <bill.fischo...@linaro.org <mailto:bill.fischo...@linaro.org>>
> wrote:
> > > On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
> > > <ricardo.salv...@linaro.org <mailto:ricardo.salv...@linaro.org>>
> > wrote:
> > >>
> > >> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
> > >> <bill.fischo...@linaro.org <mailto:bill.fischo...@linaro.org>>
> > wrote:
> > >> >
> > >> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
> > >> > <anders.rox...@linaro.org <mailto:anders.rox...@linaro.org>>
> > >> > wrote:
> > >> >>
> > >> >> Internal functions should not be part of symbols that are
> > visible
> > >> >> outside the library. Using -fvisibility=hidden hides all
> > internal
> > >> >> functions from the public ABI.
> > >> >>
> > >> >> Suggested-by: Ricardo Salveti <ricardo.salv...@linaro.org
> > <mailto:ricardo.salv...@linaro.org>>
> > >> >> Signed-off-by: Anders Roxell <anders.rox...@linaro.org
> > <mailto:anders.rox...@linaro.org>>
> >
> >
> > >> >> ---
> > >> >>  include/odp/api/spec/align.h |  8 
> > >> >>  include/odp/api/spec/atomic.h  | 

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Anders Roxell]

On 15 April 2016 at 13:02, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolai...@nokia.com> wrote:
> Agree, that it gets ugly (for interface specification readability), if GCC
> pragmas are added into spec header files. Wouldn’t it work the same way if
> the pragmas are defined where the spec file is included. Namely e.g. here
> for the barrier:

I guess it would work, haven't tried. That means that all the implementations
have to add this into their files.
If we what we have in this patch implementations gets it for "free".

Cheers,
Anders

>
>
>
>
>
> #ifndef ODP_PLAT_BARRIER_H_
>
> #define ODP_PLAT_BARRIER_H_
>
>
>
> #ifdef __cplusplus
>
> extern "C" {
>
> #endif
>
>
>
> #include 
>
> #include 
>
> #include 
>
> #include 
>
>
>
> if __GNUC__ >= 4
> #pragma GCC visibility push(default)
> #endif
>
>
>
> #include 
>
>
>
> if __GNUC__ >= 4
> #pragma GCC visibility pop
> #endif
>
>
>
>
>
> #ifdef __cplusplus
>
> }
>
> #endif
>
>
>
> #endif
>
>
>
>
>
> -Petri
>
>
>
>
>
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
> Mike Holmes
> Sent: Friday, April 15, 2016 1:44 PM
> To: Maxim Uvarov <maxim.uva...@linaro.org>
> Cc: lng-odp <lng-odp@lists.linaro.org>
> Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible
>
>
>
> Maxim it needs to be in the spec, it is the spec that defines the ABI, if
> you dont do this then everyone implementing will have to manually mark
> hundreds of other apis as internal rather than we just export the correct
> list.
>
>
>
> At least that is my understanding at least.
>
>
>
>
>
>
>
> On 15 April 2016 at 06:38, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
>
> Petri, please review this patch. For me it's not clear why it touches
> include/odp/api/spec files at all.
>
> Maxim.
>
> On 04/13/16 22:06, Bill Fischofer wrote:
>
> Ok, thanks.  With that:
>
> Reviewed-by: Bill Fischofer <bill.fischo...@linaro.org
> <mailto:bill.fischo...@linaro.org>>
>
> On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti <ricardo.salv...@linaro.org
> <mailto:ricardo.salv...@linaro.org>> wrote:
>
> On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
> <bill.fischo...@linaro.org <mailto:bill.fischo...@linaro.org>> wrote:
> > On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
> > <ricardo.salv...@linaro.org <mailto:ricardo.salv...@linaro.org>>
> wrote:
> >>
> >> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
> >> <bill.fischo...@linaro.org <mailto:bill.fischo...@linaro.org>>
> wrote:
> >> >
> >> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
> >> > <anders.rox...@linaro.org <mailto:anders.rox...@linaro.org>>
> >> > wrote:
> >> >>
> >> >> Internal functions should not be part of symbols that are
> visible
> >> >> outside the library. Using -fvisibility=hidden hides all
> internal
> >> >> functions from the public ABI.
> >> >>
> >> >> Suggested-by: Ricardo Salveti <ricardo.salv...@linaro.org
> <mailto:ricardo.salv...@linaro.org>>
> >> >> Signed-off-by: Anders Roxell <anders.rox...@linaro.org
> <mailto:anders.rox...@linaro.org>>
>
>
> >> >> ---
> >> >>  include/odp/api/spec/align.h |  8 
> >> >>  include/odp/api/spec/atomic.h  |  8 
> >> >>  include/odp/api/spec/barrier.h |  8 
> >> >>  include/odp/api/spec/buffer.h  |  8 
> >> >>  include/odp/api/spec/byteorder.h |  8 
> >> >> include/odp/api/spec/classification.h |  8 
> >> >>  include/odp/api/spec/compiler.h  |  8 
> >> >>  include/odp/api/spec/config.h  |  8 
> >> >>  include/odp/api/spec/cpu.h |  8 
> >> >>  include/odp/api/spec/cpumask.h |  8 
> >> >>  include/odp/api/spec/crypto.h  |  8 
> >> >>  include/odp/api/spec/debug.h |  8 
> >> >>  include/odp/api/spec/errno.h |  8 
> >> >>  include/odp/api/spec/event.h |  8 
> >> >>  include/odp/api/spec/hash.h  |  8 +++

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Savolainen, Petri (Nokia - FI/Espoo)]

Agree, that it gets ugly (for interface specification readability), if GCC 
pragmas are added into spec header files. Wouldn’t it work the same way if the 
pragmas are defined where the spec file is included. Namely e.g. here for the 
barrier:


#ifndef ODP_PLAT_BARRIER_H_
#define ODP_PLAT_BARRIER_H_

#ifdef __cplusplus
extern "C" {
#endif

#include 
#include 
#include 
#include 

if __GNUC__ >= 4
#pragma GCC visibility push(default)
#endif

#include 

if __GNUC__ >= 4
#pragma GCC visibility pop
#endif


#ifdef __cplusplus
}
#endif

#endif


-Petri


From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Mike 
Holmes
Sent: Friday, April 15, 2016 1:44 PM
To: Maxim Uvarov <maxim.uva...@linaro.org>
Cc: lng-odp <lng-odp@lists.linaro.org>
Subject: Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

Maxim it needs to be in the spec, it is the spec that defines the ABI, if you 
dont do this then everyone implementing will have to manually mark hundreds of 
other apis as internal rather than we just export the correct list.

At least that is my understanding at least.



On 15 April 2016 at 06:38, Maxim Uvarov 
<maxim.uva...@linaro.org<mailto:maxim.uva...@linaro.org>> wrote:
Petri, please review this patch. For me it's not clear why it touches 
include/odp/api/spec files at all.

Maxim.

On 04/13/16 22:06, Bill Fischofer wrote:
Ok, thanks.  With that:

Reviewed-by: Bill Fischofer 
<bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> 
<mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>>

On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti 
<ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org> 
<mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>> wrote:

On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
<bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> 
<mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>> wrote:
> On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
> <ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org> 
<mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>>
wrote:
>>
>> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
>> <bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org> 
<mailto:bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>>
wrote:
>> >
>> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
>> > <anders.rox...@linaro.org<mailto:anders.rox...@linaro.org> 
<mailto:anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>>>
>> > wrote:
>> >>
>> >> Internal functions should not be part of symbols that are
visible
>> >> outside the library. Using -fvisibility=hidden hides all
internal
>> >> functions from the public ABI.
>> >>
>> >> Suggested-by: Ricardo Salveti 
<ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>
<mailto:ricardo.salv...@linaro.org<mailto:ricardo.salv...@linaro.org>>>
>> >> Signed-off-by: Anders Roxell 
<anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>
<mailto:anders.rox...@linaro.org<mailto:anders.rox...@linaro.org>>>

>> >> ---
>> >>  include/odp/api/spec/align.h |  8 
>> >>  include/odp/api/spec/atomic.h  |  8 
>> >>  include/odp/api/spec/barrier.h |  8 
>> >>  include/odp/api/spec/buffer.h  |  8 
>> >>  include/odp/api/spec/byteorder.h |  8 
>> >> include/odp/api/spec/classification.h |  8 
>> >>  include/odp/api/spec/compiler.h  |  8 
>> >>  include/odp/api/spec/config.h  |  8 
>> >>  include/odp/api/spec/cpu.h |  8 
>> >>  include/odp/api/spec/cpumask.h |  8 
>> >>  include/odp/api/spec/crypto.h  |  8 
>> >>  include/odp/api/spec/debug.h |  8 
>> >>  include/odp/api/spec/errno.h |  8 
>> >>  include/odp/api/spec/event.h |  8 
>> >>  include/odp/api/spec/hash.h  |  8 
>> >>  include/odp/api/spec/hints.h |  8 
>> >>  include/odp/api/spec/init.h  |  8 
>> >>  include/odp/api/spec/packet.h  |  8 
>> >>  include/odp/api/spec/packet_flags.h  |  8 
>> >>  include/odp/api/spec/packet_io.h 

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Mike Holmes]

Maxim it needs to be in the spec, it is the spec that defines the ABI, if
you dont do this then everyone implementing will have to manually mark
hundreds of other apis as internal rather than we just export the correct
list.

At least that is my understanding at least.



On 15 April 2016 at 06:38, Maxim Uvarov  wrote:

> Petri, please review this patch. For me it's not clear why it touches
> include/odp/api/spec files at all.
>
> Maxim.
>
> On 04/13/16 22:06, Bill Fischofer wrote:
>
>> Ok, thanks.  With that:
>>
>> Reviewed-by: Bill Fischofer  bill.fischo...@linaro.org>>
>>
>> On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti <
>> ricardo.salv...@linaro.org > wrote:
>>
>> On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
>> > wrote:
>> > On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
>> > >
>> wrote:
>> >>
>> >> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
>> >> >
>> wrote:
>> >> >
>> >> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
>> >> > >
>> >> > wrote:
>> >> >>
>> >> >> Internal functions should not be part of symbols that are
>> visible
>> >> >> outside the library. Using -fvisibility=hidden hides all
>> internal
>> >> >> functions from the public ABI.
>> >> >>
>> >> >> Suggested-by: Ricardo Salveti > >
>> >> >> Signed-off-by: Anders Roxell > >
>>
>> >> >> ---
>> >> >>  include/odp/api/spec/align.h |  8 
>> >> >>  include/odp/api/spec/atomic.h  |  8 
>> >> >>  include/odp/api/spec/barrier.h |  8 
>> >> >>  include/odp/api/spec/buffer.h  |  8 
>> >> >>  include/odp/api/spec/byteorder.h |  8 
>> >> >> include/odp/api/spec/classification.h |  8 
>> >> >>  include/odp/api/spec/compiler.h  |  8 
>> >> >>  include/odp/api/spec/config.h  |  8 
>> >> >>  include/odp/api/spec/cpu.h |  8 
>> >> >>  include/odp/api/spec/cpumask.h |  8 
>> >> >>  include/odp/api/spec/crypto.h  |  8 
>> >> >>  include/odp/api/spec/debug.h |  8 
>> >> >>  include/odp/api/spec/errno.h |  8 
>> >> >>  include/odp/api/spec/event.h |  8 
>> >> >>  include/odp/api/spec/hash.h  |  8 
>> >> >>  include/odp/api/spec/hints.h |  8 
>> >> >>  include/odp/api/spec/init.h  |  8 
>> >> >>  include/odp/api/spec/packet.h  |  8 
>> >> >>  include/odp/api/spec/packet_flags.h  |  8 
>> >> >>  include/odp/api/spec/packet_io.h |  8 
>> >> >> include/odp/api/spec/packet_io_stats.h|  8 
>> >> >>  include/odp/api/spec/pool.h  |  8 
>> >> >>  include/odp/api/spec/queue.h |  8 
>> >> >>  include/odp/api/spec/random.h  |  8 
>> >> >>  include/odp/api/spec/rwlock.h  |  8 
>> >> >> include/odp/api/spec/rwlock_recursive.h   |  8 
>> >> >>  include/odp/api/spec/schedule.h  |  8 
>> >> >> include/odp/api/spec/schedule_types.h |  8 
>> >> >>  include/odp/api/spec/shared_memory.h |  8 
>> >> >>  include/odp/api/spec/spinlock.h  |  8 
>> >> >> include/odp/api/spec/spinlock_recursive.h |  8 
>> >> >>  include/odp/api/spec/std_clib.h  |  8 
>> >> >>  include/odp/api/spec/std_types.h |  8 
>> >> >>  include/odp/api/spec/sync.h  |  8 
>> >> >>  include/odp/api/spec/system_info.h |  8 
>> >> >>  include/odp/api/spec/thread.h  |  8 
>> >> >>  include/odp/api/spec/thrmask.h |  8 
>> >> >>  include/odp/api/spec/ticketlock.h  |  8 
>> >> >>  include/odp/api/spec/time.h  |  8 
>> >> >>  include/odp/api/spec/timer.h |  8 
>> >> >>  include/odp/api/spec/traffic_mngr.h  |  8 
>> >> >>  include/odp/api/spec/version.h |  8 
>> >> >>  platform/Makefile.inc  |  1 +
>> >> >> platform/linux-generic/m4/configure.m4| 12 
>> >> >>  44 files changed, 349 insertions(+)
>> >> >>
>> >> >> diff --git a/include/odp/api/spec/align.h
>> >> >> b/include/odp/api/spec/align.h
>> >> >> index 677ff12..027b080 100644
>> >> >> --- a/include/odp/api/spec/align.h
>> >> >> +++ b/include/odp/api/spec/align.h
>> >> >> @@ -18,6 +18,10 @@
>> 

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Maxim Uvarov]

Petri, please review this patch. For me it's not clear why it touches 
include/odp/api/spec files at all.


Maxim.

On 04/13/16 22:06, Bill Fischofer wrote:

Ok, thanks.  With that:

Reviewed-by: Bill Fischofer >


On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti 
> wrote:


On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
> wrote:
> On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
> >
wrote:
>>
>> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
>> >
wrote:
>> >
>> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
>> > >
>> > wrote:
>> >>
>> >> Internal functions should not be part of symbols that are
visible
>> >> outside the library. Using -fvisibility=hidden hides all
internal
>> >> functions from the public ABI.
>> >>
>> >> Suggested-by: Ricardo Salveti >
>> >> Signed-off-by: Anders Roxell >
>> >> ---
>> >>  include/odp/api/spec/align.h |  8 
>> >>  include/odp/api/spec/atomic.h  |  8 
>> >>  include/odp/api/spec/barrier.h |  8 
>> >>  include/odp/api/spec/buffer.h  |  8 
>> >>  include/odp/api/spec/byteorder.h |  8 
>> >> include/odp/api/spec/classification.h |  8 
>> >>  include/odp/api/spec/compiler.h  |  8 
>> >>  include/odp/api/spec/config.h  |  8 
>> >>  include/odp/api/spec/cpu.h |  8 
>> >>  include/odp/api/spec/cpumask.h |  8 
>> >>  include/odp/api/spec/crypto.h  |  8 
>> >>  include/odp/api/spec/debug.h |  8 
>> >>  include/odp/api/spec/errno.h |  8 
>> >>  include/odp/api/spec/event.h |  8 
>> >>  include/odp/api/spec/hash.h  |  8 
>> >>  include/odp/api/spec/hints.h |  8 
>> >>  include/odp/api/spec/init.h  |  8 
>> >>  include/odp/api/spec/packet.h  |  8 
>> >>  include/odp/api/spec/packet_flags.h  |  8 
>> >>  include/odp/api/spec/packet_io.h |  8 
>> >> include/odp/api/spec/packet_io_stats.h|  8 
>> >>  include/odp/api/spec/pool.h  |  8 
>> >>  include/odp/api/spec/queue.h |  8 
>> >>  include/odp/api/spec/random.h  |  8 
>> >>  include/odp/api/spec/rwlock.h  |  8 
>> >> include/odp/api/spec/rwlock_recursive.h   |  8 
>> >>  include/odp/api/spec/schedule.h  |  8 
>> >> include/odp/api/spec/schedule_types.h |  8 
>> >>  include/odp/api/spec/shared_memory.h |  8 
>> >>  include/odp/api/spec/spinlock.h  |  8 
>> >> include/odp/api/spec/spinlock_recursive.h |  8 
>> >>  include/odp/api/spec/std_clib.h  |  8 
>> >>  include/odp/api/spec/std_types.h |  8 
>> >>  include/odp/api/spec/sync.h  |  8 
>> >>  include/odp/api/spec/system_info.h |  8 
>> >>  include/odp/api/spec/thread.h  |  8 
>> >>  include/odp/api/spec/thrmask.h |  8 
>> >>  include/odp/api/spec/ticketlock.h  |  8 
>> >>  include/odp/api/spec/time.h  |  8 
>> >>  include/odp/api/spec/timer.h |  8 
>> >>  include/odp/api/spec/traffic_mngr.h  |  8 
>> >>  include/odp/api/spec/version.h |  8 
>> >>  platform/Makefile.inc  |  1 +
>> >> platform/linux-generic/m4/configure.m4| 12 
>> >>  44 files changed, 349 insertions(+)
>> >>
>> >> diff --git a/include/odp/api/spec/align.h
>> >> b/include/odp/api/spec/align.h
>> >> index 677ff12..027b080 100644
>> >> --- a/include/odp/api/spec/align.h
>> >> +++ b/include/odp/api/spec/align.h
>> >> @@ -18,6 +18,10 @@
>> >>  extern "C" {
>> >>  #endif
>> >>
>> >> +#if __GNUC__ >= 4
>> >> +#pragma GCC visibility push(default)
>> >> +#endif
>> >
>> >
>> > Do these need to be guarded?  Do we care about GCC < 4 at
this point?
>> > How
>> > does this affect clang?
>>
>> It's usually good to protect that with the supported GCC
version, but
>> then the question is if the ODP project itself would be fine to
drop
>> support for GCC < 4.
>>
>> And for clang, it doesn't affect since it also works fine with it.
>
>
> So clang exports 

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Bill Fischofer]

Ok, thanks.  With that:

Reviewed-by: Bill Fischofer 

On Wed, Apr 13, 2016 at 2:04 PM, Ricardo Salveti  wrote:

> On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
>  wrote:
> > On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
> >  wrote:
> >>
> >> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
> >>  wrote:
> >> >
> >> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
> >> > 
> >> > wrote:
> >> >>
> >> >> Internal functions should not be part of symbols that are visible
> >> >> outside the library. Using -fvisibility=hidden hides all internal
> >> >> functions from the public ABI.
> >> >>
> >> >> Suggested-by: Ricardo Salveti 
> >> >> Signed-off-by: Anders Roxell 
> >> >> ---
> >> >>  include/odp/api/spec/align.h  |  8 
> >> >>  include/odp/api/spec/atomic.h |  8 
> >> >>  include/odp/api/spec/barrier.h|  8 
> >> >>  include/odp/api/spec/buffer.h |  8 
> >> >>  include/odp/api/spec/byteorder.h  |  8 
> >> >>  include/odp/api/spec/classification.h |  8 
> >> >>  include/odp/api/spec/compiler.h   |  8 
> >> >>  include/odp/api/spec/config.h |  8 
> >> >>  include/odp/api/spec/cpu.h|  8 
> >> >>  include/odp/api/spec/cpumask.h|  8 
> >> >>  include/odp/api/spec/crypto.h |  8 
> >> >>  include/odp/api/spec/debug.h  |  8 
> >> >>  include/odp/api/spec/errno.h  |  8 
> >> >>  include/odp/api/spec/event.h  |  8 
> >> >>  include/odp/api/spec/hash.h   |  8 
> >> >>  include/odp/api/spec/hints.h  |  8 
> >> >>  include/odp/api/spec/init.h   |  8 
> >> >>  include/odp/api/spec/packet.h |  8 
> >> >>  include/odp/api/spec/packet_flags.h   |  8 
> >> >>  include/odp/api/spec/packet_io.h  |  8 
> >> >>  include/odp/api/spec/packet_io_stats.h|  8 
> >> >>  include/odp/api/spec/pool.h   |  8 
> >> >>  include/odp/api/spec/queue.h  |  8 
> >> >>  include/odp/api/spec/random.h |  8 
> >> >>  include/odp/api/spec/rwlock.h |  8 
> >> >>  include/odp/api/spec/rwlock_recursive.h   |  8 
> >> >>  include/odp/api/spec/schedule.h   |  8 
> >> >>  include/odp/api/spec/schedule_types.h |  8 
> >> >>  include/odp/api/spec/shared_memory.h  |  8 
> >> >>  include/odp/api/spec/spinlock.h   |  8 
> >> >>  include/odp/api/spec/spinlock_recursive.h |  8 
> >> >>  include/odp/api/spec/std_clib.h   |  8 
> >> >>  include/odp/api/spec/std_types.h  |  8 
> >> >>  include/odp/api/spec/sync.h   |  8 
> >> >>  include/odp/api/spec/system_info.h|  8 
> >> >>  include/odp/api/spec/thread.h |  8 
> >> >>  include/odp/api/spec/thrmask.h|  8 
> >> >>  include/odp/api/spec/ticketlock.h |  8 
> >> >>  include/odp/api/spec/time.h   |  8 
> >> >>  include/odp/api/spec/timer.h  |  8 
> >> >>  include/odp/api/spec/traffic_mngr.h   |  8 
> >> >>  include/odp/api/spec/version.h|  8 
> >> >>  platform/Makefile.inc |  1 +
> >> >>  platform/linux-generic/m4/configure.m4| 12 
> >> >>  44 files changed, 349 insertions(+)
> >> >>
> >> >> diff --git a/include/odp/api/spec/align.h
> >> >> b/include/odp/api/spec/align.h
> >> >> index 677ff12..027b080 100644
> >> >> --- a/include/odp/api/spec/align.h
> >> >> +++ b/include/odp/api/spec/align.h
> >> >> @@ -18,6 +18,10 @@
> >> >>  extern "C" {
> >> >>  #endif
> >> >>
> >> >> +#if __GNUC__ >= 4
> >> >> +#pragma GCC visibility push(default)
> >> >> +#endif
> >> >
> >> >
> >> > Do these need to be guarded?  Do we care about GCC < 4 at this point?
> >> > How
> >> > does this affect clang?
> >>
> >> It's usually good to protect that with the supported GCC version, but
> >> then the question is if the ODP project itself would be fine to drop
> >> support for GCC < 4.
> >>
> >> And for clang, it doesn't affect since it also works fine with it.
> >
> >
> > So clang exports symbol __GNUC__ with a value of at least 4?
>
> Yes,
> https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/InitPreprocessor.cpp#L494
>
> Added quite a while ago.
>
> Cheers,
> --
> Ricardo Salveti
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Ricardo Salveti]

On Wed, Apr 13, 2016 at 3:51 PM, Bill Fischofer
 wrote:
> On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti
>  wrote:
>>
>> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
>>  wrote:
>> >
>> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell
>> > 
>> > wrote:
>> >>
>> >> Internal functions should not be part of symbols that are visible
>> >> outside the library. Using -fvisibility=hidden hides all internal
>> >> functions from the public ABI.
>> >>
>> >> Suggested-by: Ricardo Salveti 
>> >> Signed-off-by: Anders Roxell 
>> >> ---
>> >>  include/odp/api/spec/align.h  |  8 
>> >>  include/odp/api/spec/atomic.h |  8 
>> >>  include/odp/api/spec/barrier.h|  8 
>> >>  include/odp/api/spec/buffer.h |  8 
>> >>  include/odp/api/spec/byteorder.h  |  8 
>> >>  include/odp/api/spec/classification.h |  8 
>> >>  include/odp/api/spec/compiler.h   |  8 
>> >>  include/odp/api/spec/config.h |  8 
>> >>  include/odp/api/spec/cpu.h|  8 
>> >>  include/odp/api/spec/cpumask.h|  8 
>> >>  include/odp/api/spec/crypto.h |  8 
>> >>  include/odp/api/spec/debug.h  |  8 
>> >>  include/odp/api/spec/errno.h  |  8 
>> >>  include/odp/api/spec/event.h  |  8 
>> >>  include/odp/api/spec/hash.h   |  8 
>> >>  include/odp/api/spec/hints.h  |  8 
>> >>  include/odp/api/spec/init.h   |  8 
>> >>  include/odp/api/spec/packet.h |  8 
>> >>  include/odp/api/spec/packet_flags.h   |  8 
>> >>  include/odp/api/spec/packet_io.h  |  8 
>> >>  include/odp/api/spec/packet_io_stats.h|  8 
>> >>  include/odp/api/spec/pool.h   |  8 
>> >>  include/odp/api/spec/queue.h  |  8 
>> >>  include/odp/api/spec/random.h |  8 
>> >>  include/odp/api/spec/rwlock.h |  8 
>> >>  include/odp/api/spec/rwlock_recursive.h   |  8 
>> >>  include/odp/api/spec/schedule.h   |  8 
>> >>  include/odp/api/spec/schedule_types.h |  8 
>> >>  include/odp/api/spec/shared_memory.h  |  8 
>> >>  include/odp/api/spec/spinlock.h   |  8 
>> >>  include/odp/api/spec/spinlock_recursive.h |  8 
>> >>  include/odp/api/spec/std_clib.h   |  8 
>> >>  include/odp/api/spec/std_types.h  |  8 
>> >>  include/odp/api/spec/sync.h   |  8 
>> >>  include/odp/api/spec/system_info.h|  8 
>> >>  include/odp/api/spec/thread.h |  8 
>> >>  include/odp/api/spec/thrmask.h|  8 
>> >>  include/odp/api/spec/ticketlock.h |  8 
>> >>  include/odp/api/spec/time.h   |  8 
>> >>  include/odp/api/spec/timer.h  |  8 
>> >>  include/odp/api/spec/traffic_mngr.h   |  8 
>> >>  include/odp/api/spec/version.h|  8 
>> >>  platform/Makefile.inc |  1 +
>> >>  platform/linux-generic/m4/configure.m4| 12 
>> >>  44 files changed, 349 insertions(+)
>> >>
>> >> diff --git a/include/odp/api/spec/align.h
>> >> b/include/odp/api/spec/align.h
>> >> index 677ff12..027b080 100644
>> >> --- a/include/odp/api/spec/align.h
>> >> +++ b/include/odp/api/spec/align.h
>> >> @@ -18,6 +18,10 @@
>> >>  extern "C" {
>> >>  #endif
>> >>
>> >> +#if __GNUC__ >= 4
>> >> +#pragma GCC visibility push(default)
>> >> +#endif
>> >
>> >
>> > Do these need to be guarded?  Do we care about GCC < 4 at this point?
>> > How
>> > does this affect clang?
>>
>> It's usually good to protect that with the supported GCC version, but
>> then the question is if the ODP project itself would be fine to drop
>> support for GCC < 4.
>>
>> And for clang, it doesn't affect since it also works fine with it.
>
>
> So clang exports symbol __GNUC__ with a value of at least 4?

Yes, 
https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/InitPreprocessor.cpp#L494

Added quite a while ago.

Cheers,
-- 
Ricardo Salveti
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Bill Fischofer]

On Wed, Apr 13, 2016 at 1:47 PM, Ricardo Salveti  wrote:

> On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
>  wrote:
> >
> > On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell <
> anders.rox...@linaro.org>
> > wrote:
> >>
> >> Internal functions should not be part of symbols that are visible
> >> outside the library. Using -fvisibility=hidden hides all internal
> >> functions from the public ABI.
> >>
> >> Suggested-by: Ricardo Salveti 
> >> Signed-off-by: Anders Roxell 
> >> ---
> >>  include/odp/api/spec/align.h  |  8 
> >>  include/odp/api/spec/atomic.h |  8 
> >>  include/odp/api/spec/barrier.h|  8 
> >>  include/odp/api/spec/buffer.h |  8 
> >>  include/odp/api/spec/byteorder.h  |  8 
> >>  include/odp/api/spec/classification.h |  8 
> >>  include/odp/api/spec/compiler.h   |  8 
> >>  include/odp/api/spec/config.h |  8 
> >>  include/odp/api/spec/cpu.h|  8 
> >>  include/odp/api/spec/cpumask.h|  8 
> >>  include/odp/api/spec/crypto.h |  8 
> >>  include/odp/api/spec/debug.h  |  8 
> >>  include/odp/api/spec/errno.h  |  8 
> >>  include/odp/api/spec/event.h  |  8 
> >>  include/odp/api/spec/hash.h   |  8 
> >>  include/odp/api/spec/hints.h  |  8 
> >>  include/odp/api/spec/init.h   |  8 
> >>  include/odp/api/spec/packet.h |  8 
> >>  include/odp/api/spec/packet_flags.h   |  8 
> >>  include/odp/api/spec/packet_io.h  |  8 
> >>  include/odp/api/spec/packet_io_stats.h|  8 
> >>  include/odp/api/spec/pool.h   |  8 
> >>  include/odp/api/spec/queue.h  |  8 
> >>  include/odp/api/spec/random.h |  8 
> >>  include/odp/api/spec/rwlock.h |  8 
> >>  include/odp/api/spec/rwlock_recursive.h   |  8 
> >>  include/odp/api/spec/schedule.h   |  8 
> >>  include/odp/api/spec/schedule_types.h |  8 
> >>  include/odp/api/spec/shared_memory.h  |  8 
> >>  include/odp/api/spec/spinlock.h   |  8 
> >>  include/odp/api/spec/spinlock_recursive.h |  8 
> >>  include/odp/api/spec/std_clib.h   |  8 
> >>  include/odp/api/spec/std_types.h  |  8 
> >>  include/odp/api/spec/sync.h   |  8 
> >>  include/odp/api/spec/system_info.h|  8 
> >>  include/odp/api/spec/thread.h |  8 
> >>  include/odp/api/spec/thrmask.h|  8 
> >>  include/odp/api/spec/ticketlock.h |  8 
> >>  include/odp/api/spec/time.h   |  8 
> >>  include/odp/api/spec/timer.h  |  8 
> >>  include/odp/api/spec/traffic_mngr.h   |  8 
> >>  include/odp/api/spec/version.h|  8 
> >>  platform/Makefile.inc |  1 +
> >>  platform/linux-generic/m4/configure.m4| 12 
> >>  44 files changed, 349 insertions(+)
> >>
> >> diff --git a/include/odp/api/spec/align.h b/include/odp/api/spec/align.h
> >> index 677ff12..027b080 100644
> >> --- a/include/odp/api/spec/align.h
> >> +++ b/include/odp/api/spec/align.h
> >> @@ -18,6 +18,10 @@
> >>  extern "C" {
> >>  #endif
> >>
> >> +#if __GNUC__ >= 4
> >> +#pragma GCC visibility push(default)
> >> +#endif
> >
> >
> > Do these need to be guarded?  Do we care about GCC < 4 at this point? How
> > does this affect clang?
>
> It's usually good to protect that with the supported GCC version, but
> then the question is if the ODP project itself would be fine to drop
> support for GCC < 4.
>
> And for clang, it doesn't affect since it also works fine with it.
>

So clang exports symbol __GNUC__ with a value of at least 4?


>
> Cheers,
> --
> Ricardo Salveti
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Ricardo Salveti]

On Wed, Apr 13, 2016 at 2:47 PM, Bill Fischofer
 wrote:
>
> On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell 
> wrote:
>>
>> Internal functions should not be part of symbols that are visible
>> outside the library. Using -fvisibility=hidden hides all internal
>> functions from the public ABI.
>>
>> Suggested-by: Ricardo Salveti 
>> Signed-off-by: Anders Roxell 
>> ---
>>  include/odp/api/spec/align.h  |  8 
>>  include/odp/api/spec/atomic.h |  8 
>>  include/odp/api/spec/barrier.h|  8 
>>  include/odp/api/spec/buffer.h |  8 
>>  include/odp/api/spec/byteorder.h  |  8 
>>  include/odp/api/spec/classification.h |  8 
>>  include/odp/api/spec/compiler.h   |  8 
>>  include/odp/api/spec/config.h |  8 
>>  include/odp/api/spec/cpu.h|  8 
>>  include/odp/api/spec/cpumask.h|  8 
>>  include/odp/api/spec/crypto.h |  8 
>>  include/odp/api/spec/debug.h  |  8 
>>  include/odp/api/spec/errno.h  |  8 
>>  include/odp/api/spec/event.h  |  8 
>>  include/odp/api/spec/hash.h   |  8 
>>  include/odp/api/spec/hints.h  |  8 
>>  include/odp/api/spec/init.h   |  8 
>>  include/odp/api/spec/packet.h |  8 
>>  include/odp/api/spec/packet_flags.h   |  8 
>>  include/odp/api/spec/packet_io.h  |  8 
>>  include/odp/api/spec/packet_io_stats.h|  8 
>>  include/odp/api/spec/pool.h   |  8 
>>  include/odp/api/spec/queue.h  |  8 
>>  include/odp/api/spec/random.h |  8 
>>  include/odp/api/spec/rwlock.h |  8 
>>  include/odp/api/spec/rwlock_recursive.h   |  8 
>>  include/odp/api/spec/schedule.h   |  8 
>>  include/odp/api/spec/schedule_types.h |  8 
>>  include/odp/api/spec/shared_memory.h  |  8 
>>  include/odp/api/spec/spinlock.h   |  8 
>>  include/odp/api/spec/spinlock_recursive.h |  8 
>>  include/odp/api/spec/std_clib.h   |  8 
>>  include/odp/api/spec/std_types.h  |  8 
>>  include/odp/api/spec/sync.h   |  8 
>>  include/odp/api/spec/system_info.h|  8 
>>  include/odp/api/spec/thread.h |  8 
>>  include/odp/api/spec/thrmask.h|  8 
>>  include/odp/api/spec/ticketlock.h |  8 
>>  include/odp/api/spec/time.h   |  8 
>>  include/odp/api/spec/timer.h  |  8 
>>  include/odp/api/spec/traffic_mngr.h   |  8 
>>  include/odp/api/spec/version.h|  8 
>>  platform/Makefile.inc |  1 +
>>  platform/linux-generic/m4/configure.m4| 12 
>>  44 files changed, 349 insertions(+)
>>
>> diff --git a/include/odp/api/spec/align.h b/include/odp/api/spec/align.h
>> index 677ff12..027b080 100644
>> --- a/include/odp/api/spec/align.h
>> +++ b/include/odp/api/spec/align.h
>> @@ -18,6 +18,10 @@
>>  extern "C" {
>>  #endif
>>
>> +#if __GNUC__ >= 4
>> +#pragma GCC visibility push(default)
>> +#endif
>
>
> Do these need to be guarded?  Do we care about GCC < 4 at this point? How
> does this affect clang?

It's usually good to protect that with the supported GCC version, but
then the question is if the ODP project itself would be fine to drop
support for GCC < 4.

And for clang, it doesn't affect since it also works fine with it.

Cheers,
-- 
Ricardo Salveti
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Bill Fischofer]

For reference, GCC 4.0 was released in April 2005.

On Wed, Apr 13, 2016 at 12:47 PM, Bill Fischofer 
wrote:

>
> On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell 
> wrote:
>
>> Internal functions should not be part of symbols that are visible
>> outside the library. Using -fvisibility=hidden hides all internal
>> functions from the public ABI.
>>
>> Suggested-by: Ricardo Salveti 
>> Signed-off-by: Anders Roxell 
>> ---
>>  include/odp/api/spec/align.h  |  8 
>>  include/odp/api/spec/atomic.h |  8 
>>  include/odp/api/spec/barrier.h|  8 
>>  include/odp/api/spec/buffer.h |  8 
>>  include/odp/api/spec/byteorder.h  |  8 
>>  include/odp/api/spec/classification.h |  8 
>>  include/odp/api/spec/compiler.h   |  8 
>>  include/odp/api/spec/config.h |  8 
>>  include/odp/api/spec/cpu.h|  8 
>>  include/odp/api/spec/cpumask.h|  8 
>>  include/odp/api/spec/crypto.h |  8 
>>  include/odp/api/spec/debug.h  |  8 
>>  include/odp/api/spec/errno.h  |  8 
>>  include/odp/api/spec/event.h  |  8 
>>  include/odp/api/spec/hash.h   |  8 
>>  include/odp/api/spec/hints.h  |  8 
>>  include/odp/api/spec/init.h   |  8 
>>  include/odp/api/spec/packet.h |  8 
>>  include/odp/api/spec/packet_flags.h   |  8 
>>  include/odp/api/spec/packet_io.h  |  8 
>>  include/odp/api/spec/packet_io_stats.h|  8 
>>  include/odp/api/spec/pool.h   |  8 
>>  include/odp/api/spec/queue.h  |  8 
>>  include/odp/api/spec/random.h |  8 
>>  include/odp/api/spec/rwlock.h |  8 
>>  include/odp/api/spec/rwlock_recursive.h   |  8 
>>  include/odp/api/spec/schedule.h   |  8 
>>  include/odp/api/spec/schedule_types.h |  8 
>>  include/odp/api/spec/shared_memory.h  |  8 
>>  include/odp/api/spec/spinlock.h   |  8 
>>  include/odp/api/spec/spinlock_recursive.h |  8 
>>  include/odp/api/spec/std_clib.h   |  8 
>>  include/odp/api/spec/std_types.h  |  8 
>>  include/odp/api/spec/sync.h   |  8 
>>  include/odp/api/spec/system_info.h|  8 
>>  include/odp/api/spec/thread.h |  8 
>>  include/odp/api/spec/thrmask.h|  8 
>>  include/odp/api/spec/ticketlock.h |  8 
>>  include/odp/api/spec/time.h   |  8 
>>  include/odp/api/spec/timer.h  |  8 
>>  include/odp/api/spec/traffic_mngr.h   |  8 
>>  include/odp/api/spec/version.h|  8 
>>  platform/Makefile.inc |  1 +
>>  platform/linux-generic/m4/configure.m4| 12 
>>  44 files changed, 349 insertions(+)
>>
>> diff --git a/include/odp/api/spec/align.h b/include/odp/api/spec/align.h
>> index 677ff12..027b080 100644
>> --- a/include/odp/api/spec/align.h
>> +++ b/include/odp/api/spec/align.h
>> @@ -18,6 +18,10 @@
>>  extern "C" {
>>  #endif
>>
>> +#if __GNUC__ >= 4
>> +#pragma GCC visibility push(default)
>> +#endif
>>
>
> Do these need to be guarded?  Do we care about GCC < 4 at this point? How
> does this affect clang?
>
>
>> +
>>  /** @addtogroup odp_compiler_optim
>>   *  Macros that allow cache line size configuration, check that
>>   *  alignment is a power of two etc.
>> @@ -70,6 +74,10 @@ extern "C" {
>>   * @}
>>   */
>>
>> +#if __GNUC__ >= 4
>> +#pragma GCC visibility pop
>> +#endif
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> diff --git a/include/odp/api/spec/atomic.h b/include/odp/api/spec/atomic.h
>> index a16d90b..b926964 100644
>> --- a/include/odp/api/spec/atomic.h
>> +++ b/include/odp/api/spec/atomic.h
>> @@ -18,6 +18,10 @@
>>  extern "C" {
>>  #endif
>>
>> +#if __GNUC__ >= 4
>> +#pragma GCC visibility push(default)
>> +#endif
>> +
>>  /**
>>   * @defgroup odp_atomic ODP ATOMIC
>>   * @details
>> @@ -624,6 +628,10 @@ int odp_atomic_lock_free_u64(odp_atomic_op_t
>> *atomic_op);
>>   * @}
>>   */
>>
>> +#if __GNUC__ >= 4
>> +#pragma GCC visibility pop
>> +#endif
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> diff --git a/include/odp/api/spec/barrier.h
>> b/include/odp/api/spec/barrier.h
>> index 823eae6..34c3658 100644
>> --- a/include/odp/api/spec/barrier.h
>> +++ b/include/odp/api/spec/barrier.h
>> @@ -18,6 +18,10 @@
>>  extern "C" {
>>  #endif
>>
>> +#if __GNUC__ >= 4
>> +#pragma GCC visibility push(default)
>> +#endif
>> +
>>  /**
>>   * @defgroup odp_barrier ODP BARRIER
>>   * Thread excution and memory ordering barriers.
>> @@ -59,6 +63,10 

Re: [lng-odp] [API-NEXT PATCHv4] api: make only the API visible

[Bill Fischofer]

On Wed, Apr 13, 2016 at 12:30 PM, Anders Roxell 
wrote:

> Internal functions should not be part of symbols that are visible
> outside the library. Using -fvisibility=hidden hides all internal
> functions from the public ABI.
>
> Suggested-by: Ricardo Salveti 
> Signed-off-by: Anders Roxell 
> ---
>  include/odp/api/spec/align.h  |  8 
>  include/odp/api/spec/atomic.h |  8 
>  include/odp/api/spec/barrier.h|  8 
>  include/odp/api/spec/buffer.h |  8 
>  include/odp/api/spec/byteorder.h  |  8 
>  include/odp/api/spec/classification.h |  8 
>  include/odp/api/spec/compiler.h   |  8 
>  include/odp/api/spec/config.h |  8 
>  include/odp/api/spec/cpu.h|  8 
>  include/odp/api/spec/cpumask.h|  8 
>  include/odp/api/spec/crypto.h |  8 
>  include/odp/api/spec/debug.h  |  8 
>  include/odp/api/spec/errno.h  |  8 
>  include/odp/api/spec/event.h  |  8 
>  include/odp/api/spec/hash.h   |  8 
>  include/odp/api/spec/hints.h  |  8 
>  include/odp/api/spec/init.h   |  8 
>  include/odp/api/spec/packet.h |  8 
>  include/odp/api/spec/packet_flags.h   |  8 
>  include/odp/api/spec/packet_io.h  |  8 
>  include/odp/api/spec/packet_io_stats.h|  8 
>  include/odp/api/spec/pool.h   |  8 
>  include/odp/api/spec/queue.h  |  8 
>  include/odp/api/spec/random.h |  8 
>  include/odp/api/spec/rwlock.h |  8 
>  include/odp/api/spec/rwlock_recursive.h   |  8 
>  include/odp/api/spec/schedule.h   |  8 
>  include/odp/api/spec/schedule_types.h |  8 
>  include/odp/api/spec/shared_memory.h  |  8 
>  include/odp/api/spec/spinlock.h   |  8 
>  include/odp/api/spec/spinlock_recursive.h |  8 
>  include/odp/api/spec/std_clib.h   |  8 
>  include/odp/api/spec/std_types.h  |  8 
>  include/odp/api/spec/sync.h   |  8 
>  include/odp/api/spec/system_info.h|  8 
>  include/odp/api/spec/thread.h |  8 
>  include/odp/api/spec/thrmask.h|  8 
>  include/odp/api/spec/ticketlock.h |  8 
>  include/odp/api/spec/time.h   |  8 
>  include/odp/api/spec/timer.h  |  8 
>  include/odp/api/spec/traffic_mngr.h   |  8 
>  include/odp/api/spec/version.h|  8 
>  platform/Makefile.inc |  1 +
>  platform/linux-generic/m4/configure.m4| 12 
>  44 files changed, 349 insertions(+)
>
> diff --git a/include/odp/api/spec/align.h b/include/odp/api/spec/align.h
> index 677ff12..027b080 100644
> --- a/include/odp/api/spec/align.h
> +++ b/include/odp/api/spec/align.h
> @@ -18,6 +18,10 @@
>  extern "C" {
>  #endif
>
> +#if __GNUC__ >= 4
> +#pragma GCC visibility push(default)
> +#endif
>

Do these need to be guarded?  Do we care about GCC < 4 at this point? How
does this affect clang?


> +
>  /** @addtogroup odp_compiler_optim
>   *  Macros that allow cache line size configuration, check that
>   *  alignment is a power of two etc.
> @@ -70,6 +74,10 @@ extern "C" {
>   * @}
>   */
>
> +#if __GNUC__ >= 4
> +#pragma GCC visibility pop
> +#endif
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/include/odp/api/spec/atomic.h b/include/odp/api/spec/atomic.h
> index a16d90b..b926964 100644
> --- a/include/odp/api/spec/atomic.h
> +++ b/include/odp/api/spec/atomic.h
> @@ -18,6 +18,10 @@
>  extern "C" {
>  #endif
>
> +#if __GNUC__ >= 4
> +#pragma GCC visibility push(default)
> +#endif
> +
>  /**
>   * @defgroup odp_atomic ODP ATOMIC
>   * @details
> @@ -624,6 +628,10 @@ int odp_atomic_lock_free_u64(odp_atomic_op_t
> *atomic_op);
>   * @}
>   */
>
> +#if __GNUC__ >= 4
> +#pragma GCC visibility pop
> +#endif
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/include/odp/api/spec/barrier.h
> b/include/odp/api/spec/barrier.h
> index 823eae6..34c3658 100644
> --- a/include/odp/api/spec/barrier.h
> +++ b/include/odp/api/spec/barrier.h
> @@ -18,6 +18,10 @@
>  extern "C" {
>  #endif
>
> +#if __GNUC__ >= 4
> +#pragma GCC visibility push(default)
> +#endif
> +
>  /**
>   * @defgroup odp_barrier ODP BARRIER
>   * Thread excution and memory ordering barriers.
> @@ -59,6 +63,10 @@ void odp_barrier_wait(odp_barrier_t *barr);
>   * @}
>   */
>
> +#if __GNUC__ >= 4
> +#pragma GCC visibility pop
> +#endif
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/include/odp/api/spec/buffer.h b/include/odp/api/spec/buffer.h
> index 6631f47..caa2cb6