This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On 2016-10-18 21:11, Pedro Alves wrote:
@@ -457,12 +456,13 @@ execute_control_command (struct command_line *cmd)switch (cmd->control_type) { case simple_control: - /* A simple command, execute it and return. */ - new_line = insert_args (cmd->line); - make_cleanup (free_current_contents, &new_line); - execute_command (new_line, 0); - ret = cmd->control_type; - break; + { + /* A simple command, execute it and return. */ + std::string new_line = insert_args (cmd->line); + execute_command (&new_line[0], 0);
The need to use &new_line[0] instead of .c_str() here looks (or smells) like a code smell to me. If execute_command needs to modify the string, it should either make its own copy, or at least document in what ways it can be modified. And in general, modifying an std::string's underlying array is probably not good (if I understand the standard correctly, it's undefined behaviour, though in reality it probably always works).
Do you know of any caller of execute_command that relies on the modification that execute_command does to the passed string? If not, I think it would be safer to change the arg to a const char * and duplicate the string at function entry.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |