0

In terms of Thread Safety and general security, would the following code have anything wrong with it?

std::string & toHexString( const uint8_t *buf, uint32_t size, std::string &out ) 
{ 
  // modify 'out'
  return out; 
}

#ifndef TOHEXSTR
    #define TOHEXSTR( x, y, ) ( toHexString( x, y, std::string() ) ).c_str()
#endif

The way this will be used is to print debug statements:

printf( "Byte buffer contents: [%s].", TOHEXSTR( buf, buf_size ) );

If there is a problem with this implementation, what should be changed?

Thanks.

MarkP
  • 4,168
  • 10
  • 43
  • 84
  • Have you considered Boost.Format? –  Sep 17 '12 at 17:56
  • @Fanael: Taking on such a large dependency for something as trivial as string formatting seems like a bad idea. – Ed S. Sep 17 '12 at 18:40
  • That doesn't compile. You can't bind temporaries to non-const references. – Kerrek SB Sep 17 '12 at 18:45
  • 1
    I'm struggling to understand the purpose of the code - what does the macro wrapper add? And what's the purpose of the temporary you're passing as the third parameter to toHexString? – JoeG Sep 17 '12 at 19:01
  • The initial intent was to create a variable local to the function calling the macro. Then, by reference, pass it in to avoid copies. However, I'll follow Ethan's advice even if RVO doesn't eliminate all object copies. – MarkP Sep 17 '12 at 19:35

2 Answers2

2

Do not use a reference parameter to store output.

Simply create a local std::string inside the function and return it by value.

std::string  toHexString( const uint8_t *buf, uint32_t size ) 
{ 
  std::string out;
  // modify 'out'
  return out; 
}

Due to compiler techniques such as Return Value Optimization, this should have similar performance, but much better semantics(no need for dummy extra parameter).

As far as thread safety goes the function is probably fine. You only need to worry about thread safety when data is shared between threads, and this function should not share any data.

Bartosz Moczulski
  • 1,209
  • 9
  • 18
0

From what we can see, that function itself has no particular thread safety problems - so you'll be okay as long as the contents of the out and buf parameters aren't modified by another thread while it's running. If you use it through that macro, or similarly, then you won't have to worry about out - although as Ethan's answer notes, you are much better off writing a cleaner function and just returning the string, not mucking about with macros.

Peter
  • 7,216
  • 2
  • 34
  • 46