[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [lwip-devel] Require sys_arch_protect/unprotect to be correctly nest
From: |
Douglas |
Subject: |
Re: [lwip-devel] Require sys_arch_protect/unprotect to be correctly nested? |
Date: |
Thu, 20 Jul 2017 23:01:23 +1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 07/20/2017 08:59 PM, Simon Goldschmidt wrote:
> Douglas wrote:
>> Require sys_arch_protect/unprotect to be correctly nested?
>> [..]
>> So would a change to require sys_arch_protect/unprotect to have matching
>> nesting be considered,
>
> I'm not sure I fully understand your mail, so let me sum up how I see the
> PROTECT/UNPROTECT:
>
> To be free in implementation, we *always* need the triple of
> DECL/PROTECT/UNPROTECT meaning:
> - one can either store the old status in PROTECT and restore it in UNPROTECT
> - *or* implement a nesting count, always lock, and unlock only for the last
> nesting (in that case, DECL is unused)
> - a DECL must *not* be reused by a nested PROTECT to ensure UNPROTECT gets
> the correct level
>
> In any case, every PROTECT needs a matching UNPROTECT!
Thank you, that is basically all I was asking, and a proposed change has
been submitted https://savannah.nongnu.org/bugs/index.php?51519
>
>> perhaps even cleaning these up by removing the *DECL_PROTECT macros and the
>> levels?
>
> No, that would limit the use case to a nesting counter and prevent
> store/restore.
Ok, that seems fine to leave. It might help to clarify that the value is
opaque to lwip and just passed through, so that lwip code is not written
to use the value.
>> A quick inspection suggests one non-nested use in alloc_socket, but that
>> looks like a programming error and a patch will be submitted.
>
> I haven't fully followed this discussion. Is this sorted out now?
Yes, sorry for the noise. It was not even an issue with core locking
enabled.
But it still looks like there is a problem noted in
https://savannah.nongnu.org/bugs/index.php?51507
Btw being new to the list, my interest is the lwip port for
esp-open-rtos which is FreeRTOS/newlib/lwip for the esp8266 and my
development branch is at https://github.com/ourairquality/esp-open-rtos
Douglas