Fixed memmanager and add assertion

[Hadatko]

assertion added for functions which are returning status on freeing memory

Signed-off-by: Cervenka Dusan cervenka@acrios.com

Pull request

Choose Correct

• bug
• feature

Describe the pull request

Some functions which are freeing memory are returning statuses. These should be checked.

To Reproduce

Expected behavior

Screenshots

Desktop (please complete the following information):

• OS:
• eRPC Version:

Steps you didn't forgot to do

• I checked if other PR isn't solving this issue.
• I read Contribution details and did appropriate actions.
• PR code is tested.
• PR code is formatted.

Additional context

[Hadatko]

Hi @MichalPrincNXP i cannot test these changes but they should be valid. What do you think?

[amgross]

As I told in #275 :

1. Asserting also on NULL pointer error makes the free function behave different than standard library free, which I think is not good idea.
2. I will prefer flow will continue in case that free fails (at least at non debug flavors), it is just memory leak, not fatal.

[Hadatko]

@amgross As mentioned in same PR i was expecting you will add NULL check. But here it make sense too so i did it.
Assertion are turned ON usually for debug mode and for release mode they are off. At least it should be the case MBED and MQX port. Currently Freertos implementation seems to be in both release and debug targets, but this changes is not related to it.

[amgross]

what about the other comments?

[Hadatko]

Which one do you mean? I don't see other comment

[amgross]

[amgross]

Can't you see those code review comments in the thread?

[Hadatko]

I can't. Maybe i pushed changes after your comment and it is hidden since then. For MemManager dependency i looked here:
https://github.com/NXPmicro/mcux-sdk/blob/2661f9f314dd83a0972fb60edba9a53d61055a6f/components/mem_manager/fsl_component_mem_manager.c
and here:
https://github.com/NXPmicro/mcux-sdk/blob/2661f9f314dd83a0972fb60edba9a53d61055a6f/components/mem_manager/fsl_component_mem_manager_light.c

[amgross]

Interesting, I looked on other repository that has same functions with different inputs and return enum

[Hadatko]

@amgross i pushed commit to be backward compatible. What do you think?

[amgross]

Why do we need here backward compatibility?
Do you think someone will want to continue allocating forever?? (if yes, so you will need to remove the assert from the free I suppose because the free will fail)

[MichalPrincNXP]

Hi @Hadatko , I would remove the backward compatibility section/defines ... looking at the history of mem_manager NXP component, I am not able to find the MEM_BufferAllocForever() function (up to 2018 state). I would use MEM_BufferAllocWithId() directly.
I am also not able to verify the functionality of these two updated porting layers as these are not used in any maintained example/test.
#include "MemManager.h" should be changed to #include "fsl_component_mem_manager.h"

[Hadatko]

@MichalPrincNXP Is it ok now?

[MichalPrincNXP]

@MichalPrincNXP Is it ok now?

I think so, thank you!