Issue #1150: create a wrapper macro around sysdb_transaction_start/_end - sssd

SSSD / sssd

#1150 create a wrapper macro around sysdb_transaction_start/_end

Closed: Invalid None Opened 10 years ago by jhrozek.

We should create helper macros around sysdb_transaction_start/_commit/_end (or extend these functions) to avoid duplicating code in the common case where we track whether we are inside a transaction with a boolean variable in_transaction.

dpal commented 10 years ago

Fields changed

milestone: NEEDS_TRIAGE => SSSD Deferred
type: defect => task

dpal commented 10 years ago

Fields changed

rhbz: => 0

sbose commented 10 years ago

There are still a couple of places in the code where the following pattern is used

    if (in_transaction) {
        if (ret == EOK) {
            sret = sysdb_transaction_commit(sysdb);
            if (sret != EOK) {
                DEBUG(SSSDBG_OP_FAILURE, ("Could not commit transaction\n"));
            }
        }

        if (ret != EOK || sret != EOK){
            sret = sysdb_transaction_cancel(sysdb);
            if (sret != EOK) {
                DEBUG(SSSDBG_OP_FAILURE, ("Could not cancel transaction\n"));
            }
        }
    }

here ret is still EOK if sysdb_transaction_commit() fails. The approved pattern looks like

    ret = sysdb_transaction_commit(sysdb);
    if (ret == EOK) in_transaction = false;

done:
    if (in_transaction) {
        sret = sysdb_transaction_cancel(sysdb);
        if (sret != EOK) {
            DEBUG(SSSDBG_CRIT_FAILURE, ("Could not cancel transaction\n"));
        }
    }
    talloc_free(tmp_ctx);
    return ret;

Instead of updating the pattern it would make sense to introduce the macros described in this ticket.

feature_milestone: =>
milestone: SSSD Deferred => NEEDS_TRIAGE

dpal commented 10 years ago

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.9.0