CiscoDevNet / csmp-agent-lib

OpenCSMP: CiscoDevNet Opensource CSMP-Agent Library (Formerly CSMP-Agent)
https://github.com/CiscoDevNet/csmp-agent-lib
Apache License 2.0
5 stars 3 forks source link

Add allocator API to OSAL #12

Closed ismilak closed 5 months ago

ismilak commented 6 months ago

Add possibility to port dynamic memory allocator functions (stdlib malloc, calloc, realloc, free). With osal allocator functions, thread-safe and advanced solutions can be ported, supporting other platforms that don't use stdlib or they have other dynamic memory management implementation.

silabs-ThibautC commented 6 months ago

In addition to Istvan description: Silicon Labs would implement osal_calloc simply calling sl_calloc that is protecting the standard calloc from concurrent accesses: https://github.com/SiliconLabs/gecko_sdk/blob/gsdk_4.4/util/silicon_labs/silabs_core/memory_manager/sl_malloc.c#L56-L63

ghost commented 6 months ago

I understand the platform specific implementations for malloc, calloc, free ... etc. But why the platform specific function definitions in osal_common.h. Are these not identical for all platforms? Should not anything going into osal_common.h indeed be common across all target platforms? Should osal_common.h live in /osal with the specific target instances living in /osal/linux and /osal/freertos?

ismilak commented 6 months ago

I understand the platform specific implementations for malloc, calloc, free ... etc. But why the platform specific function definitions in osal_common.h. Are these not identical for all platforms? Should not anything going into osal_common.h indeed be common across all target platforms? Should osal_common.h live in /osal with the specific target instances living in /osal/linux and /osal/freertos?

Hi! osal_common.h is different for freertos and linux also. If the same and only one header is preferred for all platforms, the osal module architecture should be redesigned. In osal_common.h, currently there are platform related header includes (pthread, etc.) and type definitions (osal_base_type_t, osal_sem, etc.). I think it makes sense to create a separated portable header for type definitions in this case.

silabs-ThibautC commented 6 months ago

Should osal_common.h live in /osal with the specific target instances living in /osal/linux and /osal/freertos?

I think it must, yes. It is supposed to be the interface of the abstraction layer. If it contains platform-specific headers, it means that it is not fully abstracting the platform.

ismilak commented 6 months ago

Additional explanation and proposal for artifacts

osal_common
  \_ osal_common.h -> includes function declarations (and #include osal_types.h)

osal_xxxxx
  \_ osal_types.h -> includes platform specific headers and type definitons (portable)
  \_ osal.c -> includes implementation of osal functions (portable)