maria-developers team mailing list archive

[Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx>]

On Wed, 6 Jan 2016 at 21:48 Sergei Golubchik <serg@xxxxxxxxxxx> 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