APP_PWM_INSTANCE does not initialize p_cb.state

[Vincent Belaïche]

Hello,

Considering nRF5_SDK_14.2.0_17b948a.

In SDK documentation I can read that the steps to use the app_pwm library is to:

1. instantiate a context with APP_PWM_INSTANCE
2. call app_pwm_init to initialize the context

OK, now in APP_PWM_INSTANCE, the  app_pwm_cb_t m_pwm_##name##_cb structure is just instantiated, not initialized. So what you get in the state field when compiling for release is just what the RAM contains there, am I wrong ?

Then in app_pwm_init there is this test close to the beginning of the function:

     if (p_cb->state != NRF_DRV_STATE_UNINITIALIZED)

so, you test something that has not been initialized.

My questions are the following:

1. do you rely on the compiler generating some start-up code setting all static memory to zero before entering the main function ?
2. does this mean that the APP_PWM_INSTANCE macro cannot be called within a function, like in a thread --- as that would be allocated in the stack, not in the static memory ? If so, I just notice that this constraint is not documented.

  Vincent.

[Håkon Alseth]

Hi,

do you rely on the compiler generating some start-up code setting all static memory to zero before entering the main function ?

Yes. All C compliant compilers shall zero initialize the BSS area. I see your point, which is that we rely on "NRF_DRV_STATE_UNINITIALIZED" is 0. A clearer way is to set .state=NRF_DRV_STATE_UNINITIALIZED in case this enum changes order or value in the future.

does this mean that the APP_PWM_INSTANCE macro cannot be called within a function, like in a thread --- as that would be allocated in the stack, not in the static memory ? If so, I just notice that this constraint is not documented.

That is correct. The structure holds a mix of flash and ram mapped structures for both state handling and callback registration, and is required to be in the static/global scope.

Best regards,

Håkon

[Vincent Belaïche]

Dear Håkon,

Thank you for your answer. It would be good if these subtleties were documented.

In the SDK there might also be other structures that could be allocated in the stack or heap rather than in the static memory (I am just speculating), and I suspect that freeRTOS, in some configurations, may initialize these RAM areas with some non zero patterns to make it easier detect stack overflow/heap corruption.

So, you should check for that too : probably in the case of stack a different macro is needed to make the initialization explicit, and in the case of heap, you would need a completely different mechanism, so (an)other macro(s) too. As a minimal update, in any case, if you don't want to support any stack/heap allocation for structures where in theory that would be feasible, you should just clearly document it.

  Vincent.