Re: [Acl-devel] [RFC] Add attribute visibility("default") to API functions

[Yury Usishchev]

Hello!

Thank you for comments!

> we already use hidden visibility in include/misc.h, so if we want to
> invert the meaning, you'll need to update that too.
I did not notice it before, thanks:)

> do not indent the # w/cpp.  that should always be in col 0.  e.g.
> #if ...
> # define ...
> #else
> # if ........
Done.

> that said, i don't think this is the right route.  it'd be better imo:
> (1) name it ACL_API_EXPORT
Done.

> (2) just do in acl.h:
> #ifndef ACL_API_EXPORT
> # define ACL_API_EXPORT extern
> #endif
> (3) update config.h to define ACL_API_EXPORT to default visiblity
> (4) update configure.ac to test for the flag directly -- we already have
> the m4/visibility_hidden.m4 file in the codebase for this
>
> since all our source files should be including config.h first, we
> then know the macro will be defined before pulling in acl.h.
It will be better if we put it in include/misc.h like the hidden macros
And I dont think we need to add another check for visibility("default")
as it should be covered by visibility("hidden") check.

> there's no need to enumerate the definitions themselves.  just the
> prototype is sufficient.
As Andreas pointed out, information about visibility cannot be used outside
acl build, so we should not place it in public headers. Thats why I annotated
definitions.

I updated patch according to comments. Also renamed hidden define to ACL_HIDDEN
to use somewhat same style. Could you review it please?

BR,
Yury Usishchev

0001-Rework-visibility-of-symbols.patch
Description: Rework-visibility-of-symbols