Re: [lwip-devel] [bug #51990] Calling select() from different threads with MPU enabled triggers memory protection fault

[David Lockyer]

Hi Joel,

Thanks for the feedback, I'll tidy up the things you've mentioned, then 
move onto the master patchset.

I moved the #include "lwip/sys.h" from sockets.c to sockets.h in the 
submitted patchset, so it still effectively gets included. Would you 
prefer it to still be in sockets.c as well to make it look more obvious?


On 11/09/17 17:26, Joel Cunningham wrote:
> Follow-up Comment #1, bug #51990 (project lwip):
>
> David,
>
> I did an initial code review, things looked good.  Only a couple really minors
> things:
>
>
> --- a/src/api/sockets.c
> +++ b/src/api/sockets.c
> @@ -50,7 +50,6 @@
>   
>   #include "lwip/sockets.h"
>   #include "lwip/api.h"
> -#include "lwip/sys.h"
>   #include "lwip/igmp.h"
>   #include "lwip/inet.h"
>   #include "lwip/tcp.h"
>
>
> We'll want to keep lwip/sys.h as there is other code in sockets.c using things
> in the header
>
>
> @@ -77,6 +76,13 @@
>   #define LWIP_NETCONN 0
>   #endif
>   
> +#define API_SELECT_CB_VAR_REF(name)               API_VAR_REF(name)
> +#define API_SELECT_CB_VAR_DECLARE(name)           API_VAR_DECLARE(struct
> lwip_select_cb, name)
> +#define API_SELECT_CB_VAR_ALLOC(name)             API_VAR_ALLOC(struct
> lwip_select_cb, MEMP_SELECT_CB, name, ERR_MEM)
> +#define API_SELECT_CB_VAR_ALLOC_RETURN_NULL(name) API_VAR_ALLOC(struct
> lwip_select_cb, MEMP_SELECT_CB, name, NULL)
> +#define API_SELECT_CB_VAR_FREE(name)
> API_VAR_FREE(MEMP_SELECT_CB, name)
> +
> +
>
>
> Extra newline here
>
>
>   /**
> + * MEMP_NUM_SELECT_CB: the number of struct lwip_select_cb.
> + * (only needed if you use the sequential API, like api_lib.c)
> + */
> +#if !defined MEMP_NUM_SELECT_CB || defined __DOXYGEN__
> +#define MEMP_NUM_SELECT_CB              4
> +#endif
> +
> +/**
>
>
> I think for the comment on this option, we'll want to mention 'only needed if
> you use the sockets API', since select is not available when only using
> netconn API
>
> Couple notes when porting this fix to master:
> 1) We'll want to put struct lwip_select_cb and SELECT_ macros in
> sockets_priv.h rather than public API
> 2) MEMP_NUM_SELECT_CB memory pool will need to be guarded by #if
> LWIP_SOCKET_SELECT
>
>
>      _______________________________________________________
>
> Reply to this item at:
>
>    <http://savannah.nongnu.org/bugs/?51990>
>
> _______________________________________________
>    Message sent via/by Savannah
>    http://savannah.nongnu.org/
>
>
> ______________________________________________________________________
> This email has been scanned by the Symantec Email Security.cloud service.
> For more information please visit http://www.symanteccloud.com
> ______________________________________________________________________



______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________