Hi, Alexander! On Apr 01, Alexander Barkov wrote:

Hi Sergei,

Please review a patch for MDEV-9842.

This is a prerequisite for:

MDEV-6353 my_ismbchar() and my_mbcharlen() refactoring

Thanks.

commit 4d615db357a5b513c915ab677afd130a0673f0da Author: Alexander Barkov <bar@mariadb.org> Date: Fri Apr 1 14:53:00 2016 +0400

MDEV-9842 LOAD DATA INFILE does not work well with a TEXT column when using sjis Fixing READ_INFO to store the loaded data in a String object. Making sure to reserve MY_CS_MBMAXLEN bytes before loading the next character in read_field().

please, always add an empty line after the commit comment subject (first line) and the commit comment body (the rest of the comment).

diff --git a/sql/sql_load.cc b/sql/sql_load.cc index f1c2920..d7ca8ef 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -66,10 +66,9 @@ XML_TAG::XML_TAG(int l, String f, String v)

class READ_INFO { File file; - uchar *buffer, /* Buffer for read text */ - *end_of_buff; /* Data in bufferts ends here */ - uint buff_length, /* Length of buffert */ - max_length; /* Max length of row */ + String data; /* Read buffer */ + uint fixed_length; /* Length of the fixed length record */ + uint max_length; /* Max length of row */ const uchar *field_term_ptr,*line_term_ptr; const char *line_start_ptr,*line_start_end; uint field_term_length,line_term_length,enclosed_length; @@ -1349,10 +1348,11 @@ READ_INFO::READ_INFO(THD *thd, File file_par, uint tot_length, CHARSET_INFO *cs, String &field_term, String &line_start, String &line_term, String &enclosed_par, int escape, bool get_it_from_net, bool is_fifo) - :file(file_par), buffer(NULL), buff_length(tot_length), escape_char(escape), + :file(file_par), fixed_length(tot_length), escape_char(escape), found_end_of_line(false), eof(false), error(false), line_cuted(false), found_null(false), read_charset(cs) { + data.set_thread_specific(); /* Field and line terminators must be interpreted as sequence of unsigned char. Otherwise, non-ascii terminators will be negative on some platforms, @@ -1394,18 +1394,16 @@ READ_INFO::READ_INFO(THD *thd, File file_par, uint tot_length, CHARSET_INFO *cs, set_if_bigger(length,line_start.length()); stack= stack_pos= (int*) thd->alloc(sizeof(int) * length);

- if (!(buffer=(uchar*) my_malloc(buff_length+1,MYF(MY_THREAD_SPECIFIC)))) + if (data.reserve(tot_length)) error=1; /* purecov: inspected */ else { - end_of_buff=buffer+buff_length; if (init_io_cache(&cache,(get_it_from_net) ? -1 : file, 0, (get_it_from_net) ? READ_NET : (is_fifo ? READ_FIFO : READ_CACHE),0L,1, MYF(MY_WME | MY_THREAD_SPECIFIC))) { - my_free(buffer); /* purecov: inspected */ - buffer= NULL; + data.free();

do you really need to call data.free() explicitly? String destructor takes care of that automatically.

error=1; } else @@ -1428,7 +1426,7 @@ READ_INFO::READ_INFO(THD *thd, File file_par, uint tot_length, CHARSET_INFO *cs, READ_INFO::~READ_INFO() { ::end_io_cache(&cache); - my_free(buffer); + data.free();

same here

List_iterator<XML_TAG> xmlit(taglist); XML_TAG *t; while ((t= xmlit++)) @@ -1492,7 +1489,7 @@ int READ_INFO::read_field()

for (;;) { - while ( to < end_of_buff) + while (data.length() + MY_CS_MBMAXLEN < data.alloced_length()) { chr = GET; if (chr == my_b_EOF) #ifdef USE_MB - if (my_mbcharlen(read_charset, chr) > 1 && - to + my_mbcharlen(read_charset, chr) <= end_of_buff) + if (my_mbcharlen(read_charset, chr) > 1) {

The fix would've been much easier to understand if you'd put the cleanup change into a separate commit. And why did you change READ_INFO to use String at all? Regards, Sergei Chief Architect MariaDB and security@mariadb.org