Add enable/disable cb function in mem_trace

[kegilbert]

Description

This came up in: #7888

This adds a slightly cleaner method to enabling/disabling the memory tracing callbacks as outlined the above issue.

Tested with:

#include "mbed.h"                                                
#include "mbed_mem_trace.h"                                      
                                                                 
int main() {                                                     
    printf("RUNNING....\r\n");                                   
                                                                 
    mbed_mem_trace_set_callback(mbed_mem_trace_default_callback);
    uint16_t count = 0;                                          
                                                                 
    while(true) {                                                
        void *p = malloc(50);                                    
        wait(0.5f);                                              
        free(p);                                                 
        count += 1;                                              
                                                                 
        if ((count % 5) == 0) {                                  
            if (count % 10 != 0) {                               
                printf("Disabling cb in main...\r\n");           
                mbed_mem_trace_disable();                        
            } else {                                             
                printf("Setting cb to default...\r\n");          
                mbed_mem_trace_enable();                         
            }                                                    
        }                                                        
    }                                                            
}                                                                

@deepikabhavnani

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

[deepikabhavnani]

Will it be interesting to check it in multi threaded system, when some thread trying to activate it and other disable it. I am not sure if that is a valid use case or not?

[kegilbert]

@deepikabhavnani Good point! Updated it by adding the mutex locks used around malloc calls in the enable/disable functions. Do you think that'd be sufficient? Writing up a quick test in the meantime. Will squash the commits later if it works out fine.

[kegilbert]

There still could be the concern with someone using different callbacks in different threads while enabling/disabling logging output as two subsequent disables would wipe out the first cb state. Added a change to check for the current cb pointer to see if it was already null and not to disable twice. This would cause the second disable to lose its state however. Wanted to allow an internal storage mechanism for the original cb to prevent needing to have the user maintain the pointer and pass it around between threads potentially but this does add some complexity to corner cases like this.

[kegilbert]

I'll also add a check on the currently set cb to ensure it's null before overwriting it with the reserve to stop the following:

Set_cb(...) 
Enable() 

[deepikabhavnani]

Looks good to me 👍

[NirSonnenschein]

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 3220
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7948/

Triggering tests

/morph test
/morph mbed2-build

[mbed-ci]

Test : FAILURE

Build number : 3023
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7948/3023

[NirSonnenschein]

/morph test

[studavekar]

/morph test

[mbed-ci]

Test : SUCCESS

Build number : 3072
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7948/3072

[NirSonnenschein]

/morph export-build

[mbed-ci]

Exporter Build : SUCCESS

Build number : 2888
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7948/

[NirSonnenschein]

passed CI , but still needs maintainer review. @0xc0170 @cmonr @adbridge anyone have time to review?

[adbridge]

'If (mem_trace_cb == 0)' this is inconsistent with the previous function where 'if (mem_trace_cb)' is used. One is checking a value directly and one is treating the variable as a logical True/False (which I assume is how it should be used). If the latter is indeed true then the checking against 0 should actually be replaced by:
if (!mem_trace_cb).

[adbridge]

@kegilbert please address comment.

[kegilbert]

@adbridge Updated

Rereview requested

[cmonr]

Note: This could probably come into 10.2.

[0xc0170]

Note: This could probably come into 10.2.

enhancement - new functionality thus 5.11 is good here.

[cmonr]

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 3393
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7948/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

[mbed-ci]

Exporter Build : SUCCESS

Build number : 3024
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7948/

[mbed-ci]

Test : SUCCESS

Build number : 3194
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7948/3194