Re: [lwip-devel] possible issue in sockets.c

[Albert Huitsing]

Hello Simon,

Sorry to bother you again,

here is my original remark regarding sockets.c :

in the function event_callback() there seems to be a potential problem with detecting possible
modification of the list while looping:

       }
       if (do_signal) {
         scb->sem_signalled = 1;
         /* Don't call SYS_ARCH_UNPROTECT() before signaling the semaphore, as this might
            lead to the select thread taking itself off the list, invalidagin the semaphore. */
         sys_sem_signal(&scb->sem);
       }
     }
     /* unlock interrupts with each step */
     last_select_cb_ctr = select_cb_ctr;


the call to sys_sem_signal() could cause a modification of the list.

so I think the 'select_cb_ctr' has to be stored *before* the call to sys_sem_signal()

your answer:

The actual protection here is done through SYS_ARCH_PROTECT(), not by the order of storing the pointer vs. semaphore signaling, so this should be OK.

I'm using FreeRTOS, and the SYS_ARCH_PROTECT() macro is mapped ultimatively to FreeRTOS's vPortEnterCritical() like in the many examples found on the net.

What I understand from FreeRTOS is that vPortEnterCritical() only blocks 'pre-emptive' task switching? But this still can cause sys_sem_signal() to switch tasks or isn't it?

you're saying that SYS_ARCH_PROTECT() should block all task switching alltogether? Is that really the intention of SYS_ARCH_PROTECT()?

I still think there is a bug inside event_callback() of sockets.c, and it's fixed by moving the 'last_select_cb_ctr' in front of the sys_sem_signal()

what do you think?


kindest regards,

Albert Huitsing

-- 
Albert Huitsing (address@hidden)
Huitsing Embedded Systems
Dr. Mondenweg 5
7831 JA  Nw. Weerdinge
+31-(0)591-521222
http://www.huitsing.nl
"conformity is the uncomfortable feeling of wearing 
 somebody else's clothes; even when they don't fit"