Hi, Monty! On Mar 31, Michael Widenius wrote:
--- a/sql/sql_string.h +++ b/sql/sql_string.h @@ -600,25 +600,34 @@ class Binary_string: public Static_binary_string
inline char *c_ptr() { - DBUG_ASSERT(!alloced || !Ptr || !Alloced_length || - (Alloced_length >= (str_length + 1))); - - if (!Ptr || Ptr[str_length]) // Should be safe - (void) realloc(str_length); + if (unlikely(!Ptr)) + return (char*) ""; + /* + Here we assume that any buffer used to initalize String has + an end \0 or have at least an accessable character at end. + This is to handle the case of String("Hello",5) efficently. + */ + if (unlikely(!alloced && !Ptr[str_length])) + return Ptr;
No, this is not good. Note the difference between
String a("Hello", 5)
and
char hello[5]; String a(buf, 5);
Your assumption should only apply to the first case, not to the second. In the first case alloced=Alloced_length=0, in the second case only alloced=0 and Alloced_length=5. So in the if() above you need to look at Alloced_length:
if (!Alloced_length && !Ptr[str_length]) return Ptr;
Unfortunately this does not work good with our code.
We have a lot of code that does things like this: String *new_str= new (thd->mem_root) String((const char*) name.str, name.length, system_charset_info) Where string is 0 terminated. With your proposed change, all of these will require a full realloc when using c_ptr().
Hmm, I don't understand. The difference between String a("Hello", 5); and char hello[5]; String a(buf, 5); is that in the first case the argument is const char*. And in that case Alloced_length=0 and in that case you can expect the string to be \0 terminated. In your example that we do "a lot", the first argument is const char*, so String will have Alloced_length==0 and my suggestion will work exactly as planned, it'll use a \0 terminated fast path. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org