On Wed, 6 Jan 2016 at 21:48 Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Vicențiu!
Hi Sergei!
On Jan 05, Vicențiu Ciorbaru wrote:
Sample code where we can avoid ifs:
case SSL_TYPE_SPECIFIED: table->field[next_field]->store(STRING_WITH_LEN("SPECIFIED"), &my_charset_latin1); table->field[next_field+1]->store("", 0, &my_charset_latin1); table->field[next_field+2]->store("", 0, &my_charset_latin1); table->field[next_field+3]->store("", 0, &my_charset_latin1); if (lex->ssl_cipher) table->field[next_field+1]->store(lex->ssl_cipher, strlen(lex->ssl_cipher), system_charset_info); if (lex->x509_issuer) table->field[next_field+2]->store(lex->x509_issuer, strlen(lex->x509_issuer), system_charset_info); if (lex->x509_subject) table->field[next_field+3]->store(lex->x509_subject, strlen(lex->x509_subject), system_charset_info);
This just becomes: case SSL_TYPE_SPECIFIED: table->field[next_field]->store(STRING_WITH_LEN("SPECIFIED"),
&my_charset_latin1);
table->field[next_field+1]->store(ssl_cipher.c_str(),
ssl_cipher.size(), &my_charset_latin1);
table->field[next_field+2]->store(x509_issuer.c_str(),
x509_issuer.size(), &my_charset_latin1);
table->field[next_field+3]->store(x509_subject.c_str(),
x509_subject.size(), &my_charset_latin1);
Thoughts?
I can immediately think of two more approaches:
1. using String class (sql/sql_string.h) 2. using safe_str helper:
table->field[next_field+1]->store(safe_str(lex->ssl_cipher), safe_strlen(lex->ssl_cipher), &my_charset_latin1);
As for std:string - currently the server code uses these approaches to storing strings: char*, LEX_STRING, DYNAMIC_STRING, CSET_STRING, String. With variations like uchar*, LEX_CSTRING, LEX_CUSTRING, StringBuffer, may be more.
I don't particularly like an idea of adding yes another alternative into this mess without a clear rule of what to use where.
That's what I feel would be an issue too.
So, could you say when should one use std::string and when one should stick to one of the existing types?
The place where using the std::string would be preferable, I'd say is where we don't make use of a memroot to automatically take care of memory allocations. This is indeed rare however. Another interesting alternative is when we copy the string a few times. We talked about the move semantics in Germany. Using a std::string might allow the compiler to optimise such copies as it has optimisations specifically tailored for std::string. Another problem with LEX_STRING and it's derivatives is that we do not actually enforce const-ness during compile time. f(const LEX_STRING* s) { s->str[1] = 'b'; } This function compiles perfectly but the const parameter does not help in detecting this bug. I agree that this kind of code clearly "smells", but it is another thing we need to watch out for. When considering ownership of a string, it is sometimes unclear if we own the string or not and you need to check the caller to understand where the string parameter actually comes from. For the use-case I mentioned, we can do just fine without std::string, but I do think that std::string could potentially replace most of our in-house strings, with the added benefit of handling memory allocations, where memroot might not be an option and having the extra type safety that comes with using it. Regards, Vicentiu