Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow setting mutex locking/unlocking callbacks before trace init. #46

Merged
merged 2 commits into from
Jul 19, 2016

Conversation

tommikas
Copy link

Calling mbed_trace_init() set the callbacks to NULL so they had to be set after initializing the library.
This left a small window of time during which traces could happen without any thread safety being in place.

Calling mbed_trace_init() set the callbacks to NULL so they had to be set after initializing the library.
This left a small window of time during which traces could happen without any thread safety being in place.
@tommikas
Copy link
Author

@RomanSaveljev @kjbracey-arm Could you review please.

@@ -162,9 +162,6 @@ int mbed_trace_init(void)
m_trace.suffix_f = 0;
m_trace.printf = mbed_trace_default_print;
m_trace.cmd_printf = 0;
m_trace.mutex_wait_f = 0;
m_trace.mutex_release_f = 0;
m_trace.mutex_lock_count = 0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, does not belong to this PR, but I would use here something like "NULL-object" pattern to avoid if (m_trace.mutex_wait_f) {...} everywhere

Copy link
Author

@tommikas tommikas Jul 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that would be a good idea, but for another PR indeed.

@RomanSaveljev
Copy link

RomanSaveljev commented Jul 18, 2016

Do we need a mutex for mbed_trace_init and then a mutex for assigning mutex functions? :) I mean all this approach seems a little bit fishy to me (probably, because I am not really in the loop)..

@tommikas
Copy link
Author

Currently the mutexes are only meant to protect the actual trace calls. Initialization and configuration aren't protected.

@RomanSaveljev
Copy link

Needs to be documented carefully, because of the "creativity window" we leave open for the user.

LGTM

@tommikas
Copy link
Author

I'll add a little more documentation.

@tommikas
Copy link
Author

@SeppoTakalo Could you take a look at this please.

@SeppoTakalo
Copy link
Contributor

+1

@tommikas tommikas merged commit 23a484f into master Jul 19, 2016
@tommikas tommikas deleted the lock-func-init branch July 19, 2016 12:33
tommikas pushed a commit that referenced this pull request Jun 16, 2017
* origin/master: (22 commits)
  output/ added to gitignore list (#66)
  Added include_directories(${CMAKE_CURRENT_SOURCE_DIR}/) include to resolve compilation error for Linux. (#63)
  Ensure tr_array doesn't print <null> when len == 0 (#65)
  Add CMakeLists.txt to support PAL Tools (#61)
  Fix header documentation (#58)
  Update README.md (#57)
  Provide environment-agnostic configuration macros. (#54)
  Flash size can be limited by defining MBED_TRACE_MAX_LEVEL
  Use temp variable in mutex release loop (#52)
  Fix race condition when unlocking mutex
  Add instructions for enabling the library in mbed OS 5. (#50)
  document update. (#47)
  Allow setting mutex locking/unlocking callbacks before trace init. (#46)
  Ipv6 helpers update (#44)
  Fix disabling traces through the YOTTA_CFG_MBED_TRACE flag. (#45)
  Deprecate the misnamed YOTTA_CFG_MTRACE_TMP_LINE_LENGTH flag, which is to be removed in the next release. (#43)
  Changed dummy definitions to match the actual functions' return types (#40)
  FEA_TRACE_SUPPORT flag is always set when traces are enabled. (#39)
  Release merge to master (#38)
  Add nanostack-libservice lib to CMakeLists (#31)
  ...
@jupe jupe added this to the v1.2.2 milestone Jul 5, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants