Re: [Firebird-devel] Oh No! Coding Style Again!

[Jim S. <ja...@ne...>]

For comparison, here is a block of code from Firebird 2:

        fb_assert(!(flags & NBAK_state_in_use));
        flags |= NBAK_state_in_use;
        bool locked = false;
        // Release shared lock to prevent possible deadlocks
        if (state_lock->lck_physical != LCK_none)
            if (LCK_convert(tdbb, state_lock, LCK_EX, LCK_NO_WAIT))
                locked = true;
            else {
                LCK_release(tdbb, state_lock);
                ast_flags &= ~NBAK_state_blocking;
                backup_state = nbak_state_unknown;
            }
        if (!locked && !LCK_lock(tdbb, state_lock, LCK_EX, LCK_WAIT)) {
            flags &= ~NBAK_state_in_use; // Lock should be released at
    this point
            gds__log("Cannot lock database backup state for writing");
            // This is OK because state changing code expect it
            ERR_post(isc_lock_conflict, 0);
        }
        NBAK_TRACE(("backup state locked for writing"));


And here is the same code reformated as per Vulcan rules:

        fb_assert(!(flags & NBAK_state_in_use));
        flags |= NBAK_state_in_use;
        bool locked = false;
       
        // Release shared lock to prevent possible deadlocks
       
        if (state_lock->lck_physical != LCK_none)
            if (LCK_convert(tdbb, state_lock, LCK_EX, LCK_NO_WAIT))
                locked = true;
            else
                {
                LCK_release(tdbb, state_lock);
                ast_flags &= ~NBAK_state_blocking;
                backup_state = nbak_state_unknown;
                }
               
        if (!locked && !LCK_lock(tdbb, state_lock, LCK_EX, LCK_WAIT))
            {
            flags &= ~NBAK_state_in_use; // Lock should be released at
    this point
            gds__log("Cannot lock database backup state for writing");
           
            // This is OK because state changing code expect it
           
            ERR_post(isc_lock_conflict, 0);
            }
           
        NBAK_TRACE(("backup state locked for writing"));

Is anyone willing to argue that the first block is easier to follow?