Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

My history of C/ASM locks here in codereview:

Once you have gotten taste of assembly, there is no turning back. I don't care if I am not ready/worthy, I going to step things up a bit and be a little bit more adventures. So I went ahead and made a lock that I hope is able to handle recursions. I will without a doubt fail very painfully and the mistake is also going to be hard to spot. No, I am not pessimistic. Just like gravity works to put things down, so does my reasoning skills to me. It is up to you to give me tough love and spot my faulty reasoning and throw bananas at me!

The code in question.

// "gate_" is the namespace marker for higher level languages that might import this code... as if...
#define gate_Gate volatile int
#define gate_Pass volatile int

extern inline void
gate_Enter
(gate_Gate *gate, gate_Pass *pass)
{
    asm volatile (
        "jmp gate_Enter_check\n"     // Skip the line.
        "gate_Enter_wait:\n"         // Wait in line.
        "pause\n"                    // Wait a long time.
        "gate_Enter_check:\n"        // Now we are at the gate.
        "cmp %[lock], %[checkin]\n"  // See if our pass is any good.
        "jge gate_Enter_skip\n"      // Skip if pass >= lock.
        "mov %[lock], %%eax\n"       // eax = 1.
        "lock xchg %%eax, %[gate]\n" // Attempt to validate our pass and connect it to the gate.
        "test %%eax, %%eax\n"        // Hope for the best.
        "jnz gate_Enter_wait\n"      // There is no hope, go back in line.
        "gate_Enter_skip:\n"         // We are VIP!
        "add %[lock], %[checkin]\n"  // Checkin pass like pro!
        : [gate] "=m" (*gate), [checkin] "=m" (*pass)
        : [lock] "r" (1)
        : "eax"
    );
}

extern inline void
gate_Leave
(gate_Gate *gate, gate_Pass *pass)
{
    asm volatile (
        "cmp %[pass], %[isLast]\n"   // Have I checked in only once?
        "jg gate_Leave_skip\n"       // Skip next step if I have checked in more then once.
        "mov %[unlock], %[gate]\n"   // Close the gate because I am the last one to leave.
        "gate_Leave_skip:\n"         // Comment!
        "add %[checkout], %[pass]\n" // Checkout. This may or may not be the last time.
        : [gate] "=m" (*gate), [pass] "=m" (*pass)
        : [unlock] "r" (0), [checkout] "r" (-1), [isLast] "r" (1)
    );
}

// Example use
gate_Gate gate = 0;
gate_Pass exampleUsePass = 0;
void exampleUse(int count)
{
    printf("pass=%d\n", exampleUsePass);
    gate_Enter(&gate, &exampleUsePass);
    if (count == 3) {
        // exampleUsePass = 0; // force deadlock.
    }
    if (count < 5) {
        printf("Count = %d\n", count);
        exampleUse(++count);
    }
    gate_Leave(&gate, &exampleUsePass);
    printf("pass=%d\n", exampleUsePass);
}

exampleUse(0);

The question

The intent of this code is a lock that allows recursion. Will this lock make exampleUse both threadsafe and reentrant if we allow our self to assume the pass, exampleUsePass is only used for that function?

  • If not? what is wrong?
  • If yes, it is still wrong; isn't it?... Give me a good rant about what to consider, what is missing or about better approach. If you want to suggest libraries, please restrict your self to C. I am not experienced enough to bind C++ stuff to other languages, so I often can't use the good stuff over there :'(
share|improve this question
    
Sorry, misread the question. – Easterly Irk May 21 at 12:37
    
I just changed the question, so maybe you didn't misread but helped me to improve it ;) – user1235831 May 21 at 12:39

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.