From 6c97e85673602199f51dd4bbcb1cf93cbf47ffc9 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Thu, 20 Sep 2018 06:13:43 +0300 Subject: [PATCH] Remove valgrind warnings from Item_str_concat This warning come from a copy() operation of type: memcpy(ptr, ptr+A, B), which is safe but produces a warning when run with valgrind. To avoid the warning, I added copy_or_move() method which uses memmove() instead of memcpy(). In 10.3 the change in item_strfunc::Item_func_concat() has to be mirroed in Item_func_concat_oracle() to avoid future valgrind warnings. --- sql/item_strfunc.cc | 2 +- sql/sql_string.cc | 23 +++++++++++++++++++++-- sql/sql_string.h | 1 + 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index e332f034fb3..57a1e7ec55b 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -603,7 +603,7 @@ String *Item_func_concat::val_str(String *str) goto null; if (res != str) - str->copy(res->ptr(), res->length(), res->charset()); + str->copy_or_move(res->ptr(), res->length(), res->charset()); for (uint i= 1 ; i < arg_count ; i++) { diff --git a/sql/sql_string.cc b/sql/sql_string.cc index beff50bd1c5..615de8b545a 100644 --- a/sql/sql_string.cc +++ b/sql/sql_string.cc @@ -184,9 +184,9 @@ bool String::copy(const String &str) bool String::copy(const char *str,size_t arg_length, CHARSET_INFO *cs) { + DBUG_ASSERT(arg_length < UINT_MAX32); if (alloc(arg_length)) return TRUE; - DBUG_ASSERT(arg_length < UINT_MAX32); if (Ptr == str && arg_length == uint32(str_length)) { /* @@ -203,6 +203,24 @@ bool String::copy(const char *str,size_t arg_length, CHARSET_INFO *cs) return FALSE; } +/* + Copy string, where strings may overlap. + Same as String::copy, but use memmove instead of memcpy to avoid warnings + from valgrind +*/ + +bool String::copy_or_move(const char *str,size_t arg_length, CHARSET_INFO *cs) +{ + DBUG_ASSERT(arg_length < UINT_MAX32); + if (alloc(arg_length)) + return TRUE; + if ((str_length=uint32(arg_length))) + memmove(Ptr,str,arg_length); + Ptr[arg_length]=0; + str_charset=cs; + return FALSE; +} + /* Checks that the source string can be just copied to the destination string @@ -336,8 +354,9 @@ bool String::set_or_copy_aligned(const char *str,uint32 arg_length, /* How many bytes are in incomplete character */ uint32 offset= (arg_length % cs->mbminlen); - if (!offset) /* All characters are complete, just copy */ + if (!offset) { + /* All characters are complete, just use given string */ set(str, arg_length, cs); return FALSE; } diff --git a/sql/sql_string.h b/sql/sql_string.h index 4f86d0dd0f1..ea810f15f80 100644 --- a/sql/sql_string.h +++ b/sql/sql_string.h @@ -429,6 +429,7 @@ public: bool copy(); // Alloc string if not alloced bool copy(const String &s); // Allocate new string bool copy(const char *s,size_t arg_length, CHARSET_INFO *cs); // Allocate new string + bool copy_or_move(const char *s,size_t arg_length, CHARSET_INFO *cs); static bool needs_conversion(uint32 arg_length, CHARSET_INFO *cs_from, CHARSET_INFO *cs_to, uint32 *offset);