PR - Issue 49254 - Fix compiler failures and warnings

[389-ds-bot]

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50844

• Created at 2020-01-22 21:13:01 by mreynolds (@mreynolds389)
• Merged at 2020-01-23 18:35:08

---

Description:

Fix issues with new gcc compiler flag "-fno-common", and clean up doxygen warnings around libsds

relates: Resolves: #2313

[389-ds-bot]

Comment from spichugi (@droideck) at 2020-01-22 23:20:05

If I understand correctly, extern will only declare the variable but it still needs to be defined somewhere otherwise it can't be used

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2020-01-22 23:53:06

Yeah, I think @droideck is right here, the change to these two values could be an issue. Looking at the code I see:

./ldap/servers/plugins/acl/acllist.c:662:        if (index >= aclpb_max_selected_acls - 2) {
./ldap/servers/plugins/acl/acllist.c:722:                        (index < aclpb_max_selected_acls - 2);
./ldap/servers/plugins/acl/acllist.c:758:            if (index >= aclpb_max_selected_acls - 2) {
./ldap/servers/plugins/acl/acllist.c:848:    if ((!scan_entire_list && (*cookie >= (aclpb_max_selected_acls - 1))) ||
./ldap/servers/plugins/acl/acl.h:308: * of aclpb_max_selected_acls and aclpb_max_cache_results.
./ldap/servers/plugins/acl/acl.h:309: * If not set, DEFAULT_ACLPB_MAX_SELECTED_ACLS will be used.
./ldap/servers/plugins/acl/acl.h:311:#define ATTR_ACLPB_MAX_SELECTED_ACLS    "nsslapd-aclpb-max-selected-acls"
./ldap/servers/plugins/acl/acl.h:312:#define DEFAULT_ACLPB_MAX_SELECTED_ACLS 200
./ldap/servers/plugins/acl/acl.h:314:int aclpb_max_selected_acls; /* initialized from plugin config entry */
./ldap/servers/plugins/acl/acl_ext.c:141:        slapi_ch_calloc(aclpb_max_selected_acls, sizeof(int));
./ldap/servers/plugins/acl/acl_ext.c:366:    int value = slapi_entry_attr_get_int(e, ATTR_ACLPB_MAX_SELECTED_ACLS);
./ldap/servers/plugins/acl/acl_ext.c:368:        aclpb_max_selected_acls = value;
./ldap/servers/plugins/acl/acl_ext.c:371:        aclpb_max_selected_acls = DEFAULT_ACLPB_MAX_SELECTED_ACLS;
./ldap/servers/plugins/acl/acl_ext.c:372:        aclpb_max_cache_results = DEFAULT_ACLPB_MAX_SELECTED_ACLS;
./ldap/servers/plugins/acl/acl_ext.c:662:        slapi_ch_calloc(aclpb_max_selected_acls, sizeof(int));
./ldap/servers/plugins/acl/acl_ext.c:664:        slapi_ch_calloc(aclpb_max_selected_acls, sizeof(int));
./ldap/servers/plugins/acl/acl_ext.c:673:        slapi_ch_calloc(aclpb_max_selected_acls, sizeof(int));
./ldap/servers/plugins/acl/acl_ext.c:675:        slapi_ch_calloc(aclpb_max_selected_acls, sizeof(int));
./ldap/servers/plugins/acl/acl_ext.c:677:        slapi_ch_calloc(aclpb_max_selected_acls, sizeof(int));
./ldap/servers/plugins/acl/acl.c:1947:        while (kk < aclpb_max_selected_acls && aclpb->aclpb_handles_index[kk] != -1) {
./ldap/servers/plugins/acl/acl.c:2672:        while (kk < aclpb_max_selected_acls && aclpb->aclpb_handles_index[kk] >= 0)
./ldap/servers/plugins/acl/acl.c:2674:        if (kk >= aclpb_max_selected_acls) {
./ldap/servers/plugins/acl/acl.c:2689:        if (nhandle < aclpb_max_selected_acls) {
./ldap/servers/plugins/acl/acl.c:3105:                                  j, aclpb_max_cache_results, ATTR_ACLPB_MAX_SELECTED_ACLS);
./ldap/servers/plugins/acl/acl.c:3328:                                  j, aclpb_max_cache_results, ATTR_ACLPB_MAX_SELECTED_ACLS);

Which means the int is never defined as having a location in the programs memory as extern is only to provide a shared name I think. I think to make this valid the extern int lines need to be:

extern int aclpb_max_... = 0;

In the .h, because that will cause the compiler to allocate memory to them. How this works when it's included in multiple files though. ..... I'm not sure. Either way, I sense something going on here ...

[389-ds-bot]

Comment from mhonek (@kenoh) at 2020-01-23 00:04:53

I'm curious if we still don't need to backtrack since we are in a loop and the p is defined outside of it. I'd maybe instead of these two lines do something like *(p+1) = '"';.

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-01-23 00:39:31

Which means the int is never defined as having a location in the programs memory as extern is only to provide a shared name I think. I think to make this valid the extern int lines need to be:
extern int aclpb_max_... = 0;

This causes the build to fail with the same errors I'm trying to clean up. I'll run an ACI test or two tomorrow and see if anything arises with the current patch.

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2020-01-23 08:41:58

I agree with @kenoh suggestion those two lines sets '"' after p (and we are sure to have the enough room to do so).
Comments are precious to understand the loop but this one could be improved with something like

 /* Put in the end quote. I another snp_detail is append,  a comma will overwrite the quote*/

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2020-01-23 09:42:24

A possibility is to declare the variables in header file (extern) like you did. Then declare the effective variable in the acl_ext.c where it is set, optionally with default value 'int aclpb_max_selected_acls = DEFAULT_ACLPB_MAX_SELECTED_ACLS;'

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-01-23 15:37:40

rebased onto 6f3ae5e08f0fc759ea919185080e5985e426d4b5

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-01-23 15:38:42

Changes applied, please review...

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2020-01-23 17:36:49

Actually both variables have the same default DEFAULT_ACLPB_MAX_SELECTED_ACLS (according to acl__handle_plugin_config_entry)

[389-ds-bot]

Comment from mhonek (@kenoh) at 2020-01-23 17:52:13

Actually both variables have the same default DEFAULT_ACLPB_MAX_SELECTED_ACLS (according to acl__handle_plugin_config_entry)

I agree. Other than that, LGTM!

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-01-23 18:31:46

rebased onto 79f8351fc156807e836b5f3c9e1028007a078384

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-01-23 18:34:33

rebased onto 3c38d33

[389-ds-bot]

Comment from mreynolds (@mreynolds389) at 2020-01-23 18:35:09

Pull-Request has been merged by mreynolds389

[389-ds-bot]

Patch
50844.patch