corruption of char* in mjsonrpc.cxx/start_program

Issue #132 resolved
Thomas Lindner created an issue

Pierre Gorel found problem in mjsonrpc code. The symptom of the problem was that 'start' button wasn't working for starting programs on Programs page.

The problem seems to be with the lines

const char* name = mjsonrpc_get_param(params, "name", &error)->GetString().c_str(); if (error) return error;

std::string path = "";

in start_program(). On Pierre's setup (Ubuntu 16.04.3 LTS) the variable 'name' gets overwritten after the new 'path' variable is created and hence the wrong ODB key is searched for.

I cannot reproduce the problem on Centos-7 or Macos 10.13.

The relevant line does seem a little suspicious. The char is pointing to the memory of the std::string returned by GetString(). So if the string goes out of scope it would make sense that that char would then be pointing to invalid memory. But I would have thought that that string would not go out of scope (and be destructed) until the end of the function, so this shouldn't be a problem.

Anyway, it is probably fine to switch this function to just use std::string only. That's cleaner anyway. We will test this fix.

However, there are many other similar lines in mjsonrpc.cxx. We need to figure out if they are all problematic.

Comments (2)

  1. Thomas Lindner reporter

    The first change to mjsonrpc in start_program() fixed Pierre's problem. I also fixed a couple of other easier cases in mjsonrpc.cxx.

    There are a couple remaining similar problems in the history mjsonrpc functions. I will talk to Konstantin about how to solve these.

  2. Thomas Lindner reporter

    Made another commit, which should have fixed the remaining instances of this questionable GetString().c_str() construction.

    Long term solution would be to convert history classes to using std::strings, but that is for another day.

    This particular problem should be fixed.

  3. Log in to comment