Re: [RFC 1/8] Use std::map for MI commands in mi-cmds.c

Tom Tromey
2019-04-25T19:13:01Z

>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:

Jan> From: Didier Nadeau <didier.nadeau@gmail.com>
Jan> This changes the hashmap used in mi-cmds.c from a custom
Jan> structure to std::map. This is done to be able to add new
Jan> MI commands at runtime, the previous structure had a fixed
Jan> size array.

Thanks for doing this.

Jan> +static std::map<std::string, mi_cmd_up> mi_cmd_table;

My main concern about this approach is the lifetime of the mi_cmd.

If running an MI command can change this table, then using a unique_ptr
is dangerous, because this code hands out raw pointers which might
become invalid.

If that can't happen, then I guess that's fine. Otherwise it seems like
shared_ptr is a better choice, provided mi_cmd_lookup also returns
shared_ptr.

What do you think?

Jan> + std::string name (command->name);
Jan> +
Jan> + if (mi_cmd_table.find (name) != mi_cmd_table.end ())
Jan> + return false;
Jan> +
Jan> + mi_cmd_table[name] = std::move (command);

Can gdb::string_view be used instead?
That would avoid an allocation, and the code is already assuming that
the strings live long enough.

Tom