You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Alan Conway <ac...@redhat.com> on 2007/11/15 21:48:01 UTC

c++ tip: don't use string literals as std::string parameters in production code.

Just spotted this in a recent commit:

+    ft = FieldTable ();
+    ft.setString ("name",   "stagingThreshold");
+    ft.setInt    ("type",   TYPE_U32);
+    ft.setInt    ("access", ACCESS_RO);
+    ft.setInt    ("index",  0);
+    ft.setString ("desc",   "Broker stages messages over this size to 
disk");
+    buf.put (ft);

(It's actually not so bad because its in a rarely-called bit of code, 
but I'll highlight the general issue because it can be *extremely* bad 
in tight loops)

Every time a std::string is created from a c-string (e.g. a "literal") 
it mallocs a new copy of the string. So an innocent-looking line like 
ft.setInt("index", 0) actually causes a heap allocation every time it's 
executed. If its executed frequently the resulting heap bookkeeping can 
be very expensive.

The solution is to allocate global std::string constants for all the 
string constants you use, e.g.

namespace { const std::string INDEX("index"); }
void foo() {
   ft.setString(INDEX, 0);
   ...
}

Now the string constant gets allocated once during startup, and is 
efficiently shared by refcounting thereafter.

DON'T DO THIS!

void foo() {
   static const std::string("index");
   ...
}

It might look the same but static local variables are not thread safe 
(in general - static local POD variables are, but strings are not PODs.) 
  From personal experience this can and will core a system in production 
and is VERY hard to track down.