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

[Mike Frysinger]

On 15 Feb 2016 16:02, Yury Usishchev wrote:
> I did not fully understand your idea last time, sorry:)
> I get it now, and yes, it can work, but it will need some more changes to 
> sources.
> 
> While 'include/acl.h' is working well with new approach, I have problems with 
> declarations from 'include/libacl.h'.
> Last header is also public, and it is not included into source files during 
> build. So now I'm getting errors like
> 
> acl/build/../tools/chacl.c:275: undefined reference to `acl_entries'
> 
> Also there is an error in tools/parse.c:
> here config.h in included after acl.h:
> 
> In file included from ../include/misc.h:21:0,
> from ../tools/parse.c:37:
> ./include/config.h:5:0: warning: "ACL_API_EXPORT" redefined
> #define ACL_API_EXPORT __attribute__ ((visibility ("default"))) extern
> In file included from ../tools/parse.c:33:0:
> ./include/sys/acl.h:32:0: note: this is the location of the previous 
> definition
> # define ACL_API_EXPORT extern
> 
> Second problem is easy to fix - just move includes in tools/parse.c
> But first one will require changes to many files in libacl/ directory
> 
> For example - libacl/acl_check.c
> It includes libacl/libacl.h which includes include/acl.h. From build command 
> comes include libacl/perm_copy.h which
> includes misc.h (with config.h)
> However it does not include include/libacl.h where acl_check is declared 
> (with proper visibility attribute) therefore
> acl_check is not visible.
> 
> I can name at least 2 more such files(libacl/acl_entries.c and 
> libacl/acl_get_perm.c).
> 
> Should I fix these problems? And if yes should that fix go as separate patch 
> or same is ok?

every file should be including config.h first.  it's actually an error
that we haven't been doing this ... it just hasn't bitten us in a way
that we noticed before, so we've continued to be lazy about it :).  an
example of this being bad is that config.h does things like:
# define _ALL_SOURCE 1
# define _GNU_SOURCE 1
# define _POSIX_PTHREAD_SEMANTICS 1
# define _TANDEM_SOURCE 1
# define __EXTENSIONS__ 1

and those must come before you include any system header file in order
for them to work properly.

if you wanted to send a patch that deleted the config.h include from
the misc.h and moved it to the start of every file, that'd be fine.
do it as a sep patch so it won't be controversial :).
-mike

signature.asc
Description: Digital signature