Hi Sergei, Please review the second version, according to our discussion on IRC. Thanks. On 12/17/2015 10:15 PM, Sergei Golubchik wrote:
Hi, Alexander!
On Dec 16, Alexander Barkov wrote:
Hi Sergei,
Please review a patch for MDEV-8844.
Thanks!
diff --git a/sql/sql_error.cc b/sql/sql_error.cc index b72d642..1ed3547 100644 --- a/sql/sql_error.cc +++ b/sql/sql_error.cc @@ -931,7 +931,7 @@ char *err_conv(char *buff, uint to_length, const char *from, else { uint errors; - res= copy_and_convert(to, to_length, system_charset_info, + res= copy_and_convert(to, to_length, &my_charset_errmsg, from, from_length, from_cs, &errors);
I'm not sure I like this approach. True, when all you have is a hammer - everything looks like a nail. But we don't do *all* string conversions with charsets, do we? For example, escaping/unescaping is done with an explicit loop. I don't think my_charset_errmsg qualifies is a *charset*, it's only a helper to do map unprintable characters, it's even less charset than filename.
So I'd prefer a loop here, instead of copy_and_convert. Or, if you want to use CHARSET_INFO, then, at least
* make it hidden and not user-selectable * all properties and methods not directly related to err_conv() call should be NULL - my_charset_errmsg should not be used anywhere else.
And still, I think I'd prefer an explicit loop here.
Regards, Sergei