2009/3/4 Neil Jerram <n...@ossau.uklinux.net>: > Here's another lock ordering fix. (For 1.8.x at least, I haven't > checked if it's relevant to master yet.)
I skimmed it quickly, looks reasonable, except for this: else - SCM_OUT_OF_RANGE (2, handler); + { + SCM_CRITICAL_SECTION_END; + SCM_OUT_OF_RANGE (2, handler); + } The matching SCM_CRITICAL_SECTION_START; is not in an if block, ... before your change, where was the matching END? Why is there now an END when one did not used to be needed? Was this a bug you fixed, unrelated to the rest of the patch? In the past, without this END, there should have been a deadlock, so I guess this demonstrates that the else branch is never taken? All this is rather confusing. Now, if this was the linux kernel, people would ask you to split this patch into two patches: one to fix this unbalanced start/end problem, and another to pull the alloc out of the critical section. That makes it easier to review the correctness of the changes ... --linas