Bug#13871079 RQG_MYISAM_DML_ALTER_VALGRIND FAILS ON VALGRIND PN PB2

The class Copy_field contains a String tmp, 
which may allocate memory on the heap.
That means that all instances of Copy_field
must be properly destroyed. Alas they are not.

Solution: don't use Copy_field::tmp for copying
from_field => tmp => to_field
in do_field_string()


sql/field.cc:
  In Field_set::val_str
  return empty string (of appropriate character set) for an empty set.
sql/field.h:
  New private member in Field_enum: empty_set_string.
sql/field_conv.cc:
  In do_field_string, use an auto variable for copying
  from_field => tmp => to_field
  rather than copy->tmp.

sql/field.cc

@@ -8257,7 +8257,19 @@ String *Field_set::val_str(String *val_buffer,
   ulonglong tmp=(ulonglong) Field_enum::val_int();
   uint bitnr=0;
 
-  val_buffer->set("", 0, field_charset);
+  if (tmp == 0)
+  {
+    /*
+      Some callers expect *val_buffer to contain the result,
+      so we assign to it, rather than doing 'return &empty_set_string.
+     */
+    *val_buffer= empty_set_string;
+    return val_buffer;
+  }
+
+  val_buffer->set_charset(field_charset);
+  val_buffer->length(0);
+
   while (tmp && bitnr < (uint) typelib->count)
   {
     if (tmp & 1)

sql/field.h

@@ -1985,7 +1985,8 @@ class Field_set :public Field_enum {
     :Field_enum(ptr_arg, len_arg, null_ptr_arg, null_bit_arg,
 		    unireg_check_arg, field_name_arg,
                 packlength_arg,
-                typelib_arg,charset_arg)
+                typelib_arg,charset_arg),
+      empty_set_string("", 0, charset_arg)
     {
       flags=(flags & ~ENUM_FLAG) | SET_FLAG;
     }
@@ -1996,8 +1997,11 @@ class Field_set :public Field_enum {
   virtual bool zero_pack() const { return 1; }
   String *val_str(String*,String *);
   void sql_type(String &str) const;
+  uint size_of() const { return sizeof(*this); }
   enum_field_types real_type() const { return MYSQL_TYPE_SET; }
   bool has_charset(void) const { return TRUE; }
+private:
+  const String empty_set_string;
 };
 
 
@@ -2192,6 +2196,8 @@ class Create_field :public Sql_alloc
   {
     return (flags & (BINCMP_FLAG | BINARY_FLAG)) != 0;
   }
+private:
+  const String empty_set_string;
 };
 
 

sql/field_conv.cc

@@ -317,10 +317,11 @@ static void do_save_blob(Copy_field *copy)
 static void do_field_string(Copy_field *copy)
 {
   char buff[MAX_FIELD_WIDTH];
-  copy->tmp.set_quick(buff,sizeof(buff),copy->tmp.charset());
-  copy->from_field->val_str(&copy->tmp);
-  copy->to_field->store(copy->tmp.c_ptr_quick(),copy->tmp.length(),
-                        copy->tmp.charset());
+  String res(buff, sizeof(buff), copy->from_field->charset());
+  res.length(0U);
+
+  copy->from_field->val_str(&res);
+  copy->to_field->store(res.c_ptr_quick(), res.length(), res.charset());
 }
 
 
@@ -563,7 +564,7 @@ void Copy_field::set(uchar *to,Field *from)
 /*
   To do: 
 
-  If 'save\ is set to true and the 'from' is a blob field, do_copy is set to
+  If 'save' is set to true and the 'from' is a blob field, do_copy is set to
   do_save_blob rather than do_conv_blob.  The only differences between them
   appears to be: