Revisions to Portable C++ boost::interprocess::mutex

replaced http://stackoverflow.com/ with https://stackoverflow.com/

Source Link

edited May 23, 2017 at 12:40

Community Bot

1

I can't say for the thing as a whole, but there are a few things that you may improve:

First, this macro:
```
  #if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
  #define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
  #else
  #define IPC_DBG_TRACE(msg)
  #endif
```
It could use the do { /* ... */ } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.
This expression seems a little bit overkill:
```
  std::string mainFileName = (lockNameBase + "/main.lck").c_str(); 
```
Couldn't use simply write the following line instead? I suppose you have more chances to benefit from copy elision if you don't make several type transformations:
```
  std::string mainFileName = lockNameBase + "/main.lck";
```
I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code has undefined behaviour.
Use the standard library algorithms when you can, especially if they lower the cognitive burden. For example, instead of this conditional:
```
  _name[len < MAX_PATH ? len : MAX_PATH] = 0;
```
You can use std::min:
```
  _name[std::min(len, MAX_PATH)] = 0;
```
While it does not make a big difference, it really helps when you read it. I had to mentally parse the expression to know whether it did a min or a max while std::min makes it impossible to misread what's being done.

I can't say for the thing as a whole, but there are a few things that you may improve:

First, this macro:
```
  #if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
  #define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
  #else
  #define IPC_DBG_TRACE(msg)
  #endif
```
It could use the do { /* ... */ } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.
This expression seems a little bit overkill:
```
  std::string mainFileName = (lockNameBase + "/main.lck").c_str(); 
```
Couldn't use simply write the following line instead? I suppose you have more chances to benefit from copy elision if you don't make several type transformations:
```
  std::string mainFileName = lockNameBase + "/main.lck";
```
I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code has undefined behaviour.
Use the standard library algorithms when you can, especially if they lower the cognitive burden. For example, instead of this conditional:
```
  _name[len < MAX_PATH ? len : MAX_PATH] = 0;
```
You can use std::min:
```
  _name[std::min(len, MAX_PATH)] = 0;
```
While it does not make a big difference, it really helps when you read it. I had to mentally parse the expression to know whether it did a min or a max while std::min makes it impossible to misread what's being done.

I can't say for the thing as a whole, but there are a few things that you may improve:

First, this macro:
```
  #if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
  #define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
  #else
  #define IPC_DBG_TRACE(msg)
  #endif
```
It could use the do { /* ... */ } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.
This expression seems a little bit overkill:
```
  std::string mainFileName = (lockNameBase + "/main.lck").c_str(); 
```
Couldn't use simply write the following line instead? I suppose you have more chances to benefit from copy elision if you don't make several type transformations:
```
  std::string mainFileName = lockNameBase + "/main.lck";
```
I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code has undefined behaviour.
Use the standard library algorithms when you can, especially if they lower the cognitive burden. For example, instead of this conditional:
```
  _name[len < MAX_PATH ? len : MAX_PATH] = 0;
```
You can use std::min:
```
  _name[std::min(len, MAX_PATH)] = 0;
```
While it does not make a big difference, it really helps when you read it. I had to mentally parse the expression to know whether it did a min or a max while std::min makes it impossible to misread what's being done.

Added a note about standard library functions.

Source Link

edited Jul 8, 2015 at 8:50

Morwenn

20k
3
67
127

I can't say for the thing as a whole, but there are a few things that you may improve:

First, this macro:
```
  #if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
  #define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
  #else
  #define IPC_DBG_TRACE(msg)
  #endif
```
ThatIt could use the do { /* ... */ } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.
This expression seems a little bit too muchoverkill:
```
  std::string mainFileName = (lockNameBase + "/main.lck").c_str(); 
```
Couldn't use simply write the following line instead? I suppose you have more chances to get a workingbenefit from copy elision if you don't pass by different types.make several type transformations:
```
  std::string mainFileName = lockNameBase + "/main.lck";
```
I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code washas undefined behaviour.

Use the standard library algorithms when you can, especially if they lower the cognitive burden. For example, instead of this conditional:
```
  _name[len < MAX_PATH ? len : MAX_PATH] = 0;
```
You can use std::min:
```
  _name[std::min(len, MAX_PATH)] = 0;
```
While it does not make a big difference, it really helps when you read it. I had to mentally parse the expression to know whether it did a min or a max while std::min makes it impossible to misread what's being done.

I can't say for the thing as a whole, but there are a few things that you may improve:

First, this macro:
```
  #if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
  #define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
  #else
  #define IPC_DBG_TRACE(msg)
  #endif
```
That could use do { /* ... */ } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.
This expression seems a little bit too much:
```
  std::string mainFileName = (lockNameBase + "/main.lck").c_str(); 
```
Couldn't use simply write the following line instead? I suppose you have more chances to get a working copy elision if you don't pass by different types.
```
  std::string mainFileName = lockNameBase + "/main.lck";
```
I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code was undefined behaviour.

I can't say for the thing as a whole, but there are a few things that you may improve:

First, this macro:
```
  #if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
  #define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
  #else
  #define IPC_DBG_TRACE(msg)
  #endif
```
It could use the do { /* ... */ } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.
This expression seems a little bit overkill:
```
  std::string mainFileName = (lockNameBase + "/main.lck").c_str(); 
```
Couldn't use simply write the following line instead? I suppose you have more chances to benefit from copy elision if you don't make several type transformations:
```
  std::string mainFileName = lockNameBase + "/main.lck";
```
I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code has undefined behaviour.

Use the standard library algorithms when you can, especially if they lower the cognitive burden. For example, instead of this conditional:
```
  _name[len < MAX_PATH ? len : MAX_PATH] = 0;
```
You can use std::min:
```
  _name[std::min(len, MAX_PATH)] = 0;
```
While it does not make a big difference, it really helps when you read it. I had to mentally parse the expression to know whether it did a min or a max while std::min makes it impossible to misread what's being done.

Source Link

created Jul 28, 2014 at 14:57

Morwenn

20k
3
67
127

I can't say for the thing as a whole, but there are a few things that you may improve:

First, this macro:
```
  #if defined(_DEBUG) && defined(_ROBUST_MUTEX_TRACE) 
  #define IPC_DBG_TRACE(msg) { std::stringstream __str; __str << bipc::ipcdetail::get_current_system_highres_count() << " " << msg; xw::ipc::dbg_tracer::trace(__str); }
  #else
  #define IPC_DBG_TRACE(msg)
  #endif
```
That could use do { /* ... */ } while (0) construct to avoid some rare but nasty problems with semi-colons and compiler warnings. The answers to this question provide more information. The macro XW_IPC_HANDLE_BROKEN_STATE could also use some of those, just in case someone thinks it is a good idea to use the name oldState somewhere else.
This expression seems a little bit too much:
```
  std::string mainFileName = (lockNameBase + "/main.lck").c_str(); 
```
Couldn't use simply write the following line instead? I suppose you have more chances to get a working copy elision if you don't pass by different types.
```
  std::string mainFileName = lockNameBase + "/main.lck";
```
I know that the temporary created with operator+ should exist until the end of the expression. I don't know however whether the initialization syntax using = is part of the expression; I don't think that your code was undefined behaviour.

Stack Exchange Network

Return to Answer