Shouldn't memset structs containing std::string

Create issue
Issue #188 resolved
Ben Smith created an issue

In the new C++ midas, several structs now have std::string members (e.g. callback_addr and RPC_CLIENT_CONNECTION in msystem.h). Unfortunately, there are still places that try to use memset to initialise/reset these structures. This is undefined behaviour, and leads to crashes in my system.

Example problematic calls are:

  • mserver.cxx:306 - on my system spawned mserver processes segfault when trying to write to callback.host_name on L322, and we cannot run any remote frontends.
  • midas.cxx:10987 - on my system clients segfault when later trying to destruct the client connection object.

There are at least half a dozen other instances just for the structs in msystem.h.

I think we should define a common pattern for resetting such structures, and apply it consistently. A couple of immediately-obvious options are:

  • Define a reset() function in each struct. We would have to remember to update it whenever the struct layout changes.
  • Change them to nested structs, so the POD types are in an inner struct which can be memset. We would probably still need a reset() function to reset the strings to "", so I prefer the first option over this one.

There may be cleaner / simpler solutions I’m missing. Thoughts?

Comments (2)

  1. Ben Smith reporter

    This is now fixed in 9241f17 by implementing clear() for the relevant structs in msystem.h. There were also some places that used memcpy which I replaced with a call to the default copy-constructor.

    We should be sure to check for memset/memcpy usage for any more structs that are converted to strings in future.

  2. Log in to comment