[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Yury Usishchev
Subject: Re: [Acl-devel] [RFC] Add attribute visibility("default") to API functions
Date: Fri, 12 Feb 2016 16:39:04 +0300
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)


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 ........

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

> (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 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

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

Yury Usishchev

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]