Braces around if (op) { ... } please?

Good reminder for me to look at connection code again .... ergh.

I don't see anything obviously wrongc here, mainly just formatting :) Thanks for doing this, I know covscan fixes can be tedious

I am not saying it's wrong.
Though I'd like to understand why this change was made.

So... In other places of this file, we set IFP func = NULL; and then we put the data from pblock inside.
We also do slapi_pblock_get(pb, operation, &func) operation here, so what's the difference?

And another question, do the pblock structure somehow require to set a NULL-pointer before using slapi_pblock_set in general or in some cases?

Probably, the nitpick is very small but can we put it in one place? like done: or something

The rest looks good to me.

py.test -s -v dirsrvtests/tests/suites/basic/ dirsrvtests/tests/suites/mapping_tree/ dirsrvtests/tests/suites/replication/ dirsrvtests/tests/suites/plugins/ passes at the same rate on master as without the PR.

I am not saying it's wrong.
Though I'd like to understand why this change was made.
So... In other places of this file, we set IFP func = NULL; and then we put the data from pblock inside.
We also do slapi_pblock_get(pb, operation, &func) operation here, so what's the difference?

Good catch. I was trying to fix a warning, but this code change had no impact. I will revert it.

And another question, do the pblock structure somehow require to set a NULL-pointer before using slapi_pblock_set in general or in some cases?

You are not required to do this. Coverity complains about it sometimes though.

Probably, the nitpick is very small but can we put it in one place? like done: or something

These are actually separate functions - there is nothing really to consolidate with a "done:". Thanks for the review. I'll merge after making the suggested changes from everyone.

Probably, the nitpick is very small but can we put it in one place? like done: or something

These are actually separate functions - there is nothing really to consolidate with a "done:". Thanks for the review. I'll merge after making the suggested changes from everyone.

What I meant is that in the code:

int
dblayer_get_instance_data_dir(backend *be)
{
    ldbm_instance *inst = (ldbm_instance *)be->be_instance_info;
    char *full_namep = NULL;
    char full_name[MAXPATHLEN];
    PRDir *db_dir = NULL;
    int ret = -1;
    /* if a specific directory name was specified for this particular
     * instance use it othewise use the ldbm-wide one
     */
    full_namep = dblayer_get_full_inst_dir(inst->inst_li, inst,
                                           full_name, MAXPATHLEN);
    if (!full_namep || !*full_namep) {
        if (full_namep != full_name)
            slapi_ch_free_string(&full_namep);
        return ret;
    }
    /* Does this directory already exist? */
    if ((db_dir = PR_OpenDir(full_namep)) != NULL) {
        /* yep. */
        PR_CloseDir(db_dir);
        ret = 0;
    } else {
        /* nope -- create it. */
        ret = mkdir_p(full_namep, 0700);
    }
    if (full_name != full_namep)
        slapi_ch_free_string(&full_namep);
    return ret;
}

This part is repeated two times:

    if (full_name != full_namep)
        slapi_ch_free_string(&full_namep);
    return ret;

@spichugi - True, but TBH I don't think it justifies a "goto". Maybe if there were more occurrences but I'm just not a huge fan of goto's. In this case I'd have to goto inside that code block right to the free function or else I have to evaluate the condition test twice. If you don't mind in this case I'd prefer to leave it as is.

@spichugi - True, but TBH I don't think it justifies a "goto". Maybe if there were more occurrences but I'm just not a huge fan of goto's. In this case I'd have to goto inside that code block right to the free function or else I have to evaluate the condition test twice. If you don't mind in this case I'd prefer to leave it as is.

Okay, makes sense. Fine by me! Thanks!

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/3490

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.