389-ds-base

#50844 Issue 49254 - Fix compiler failures and warnings
Closed by spichugi. Opened by mreynolds.
mreynolds/389-ds-base fedNoCommon  into  master

Download 50844.patch

Description:

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

relates: https://pagure.io/389-ds-base/issue/49254

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

Yeah, I think @spichugi 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 ...

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) = '"';.

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.

I agree with @mhonek 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*/

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;'

rebased onto 6f3ae5e08f0fc759ea919185080e5985e426d4b5

Changes applied, please review...

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

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!

rebased onto 79f8351fc156807e836b5f3c9e1028007a078384

rebased onto 3c38d333526de2af82be095dab83ee561f6ea654

Pull-Request has been merged by mreynolds

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3898

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.