From: Sergey Glukhov
Date: April 7 2011 1:27pm
Subject: Re: [Resend] bzr commit into mysql-5.1 branch (guilhem.bichot:3645)
 Bug#11765141
List-Archive: http://lists.mysql.com/commits/135179
Message-Id: <4D9DBBC3.7050400@oracle.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
 boundary="------------080405060707040700030609"

--------------080405060707040700030609
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Guilhem,

Approved.

On 04/07/2011 05:11 PM, Guilhem Bichot wrote:
> [This commit e-mail is a repeat.]
>
> #At file:///home/mysql_src/bzrrepos_new/mysql-5.1/ based on revid:magne.mahre@stripped
>
>   3645 Guilhem Bichot	2011-04-07
>        Fix for Bug#11765141 - "58072: LOAD DATA INFILE: LEAKS IO CACHE MEMORY WHEN ERROR OCCURS"
>       @ mysql-test/t/loaddata.test
>          test for bug; without fix, running the test with --valgrind would show the leak
>          and make the test fail.
>       @ sql/sql_load.cc
>          * In READ_INFO class, 'need_end_io_cache' is true as long as init_io_cache() was called,
>          so if it's true, we need to call end_io_cache(), to free memory allocated
>          by init_io_cache(). No matter the value of 'error'. In the bug's scenario,
>          'error' was set to true in read_sep_field() because
>          '1' (read from file) isn't suitable to load into a geometric column. Because of
>          'error', end_io_cache() was not called.
>          Note: end_io_cache() calls my_b_flush_io_cache(), which will do nothing wrong given
>          that the file is opened for reads only; see the init_io_cache() call which uses
>          only those read-only types:
>          (get_it_from_net) ? READ_NET : (is_fifo ? READ_FIFO : READ_CACHE).
>          IF the cache were rather used to write to the file, my_b_flush_io_cache() may
>          write to it, and it may be questionable to write to the file
>          if 'error' is true. But here there's no problem.
>          * Now that 'need_end_io_cache' is checked even if 'error' is true, it needs
>          to be initialized in all cases.
>          * Bonus: move some variables to the initialization list.
>
>      modified:
>        mysql-test/r/loaddata.result
>        mysql-test/t/loaddata.test
>        sql/sql_load.cc
> === modified file 'mysql-test/r/loaddata.result'
> --- a/mysql-test/r/loaddata.result	2010-07-14 11:54:51 +0000
> +++ b/mysql-test/r/loaddata.result	2011-04-07 13:09:19 +0000
> @@ -532,4 +532,13 @@ a
>   0
>   1
>   DROP TABLE t1;
> +#
> +# Bug#11765141 - 58072: LOAD DATA INFILE: LEAKS IO CACHE MEMORY
> +# WHEN ERROR OCCURS
> +#
> +SELECT '1\n' INTO DUMPFILE 'MYSQLTEST_VARDIR/tmp/bug11735141.txt';
> +create table t1(a point);
> +LOAD DATA INFILE 'MYSQLTEST_VARDIR/tmp/bug11735141.txt' INTO TABLE t1;
> +ERROR 22003: Cannot get geometry object from data you send to the GEOMETRY field
> +drop table t1;
>   End of 5.1 tests
>
> === modified file 'mysql-test/t/loaddata.test'
> --- a/mysql-test/t/loaddata.test	2010-07-14 11:54:51 +0000
> +++ b/mysql-test/t/loaddata.test	2011-04-07 13:09:19 +0000
> @@ -612,4 +612,19 @@ let $MYSQLD_DATADIR= `select @@datadir`;
>   remove_file $MYSQLD_DATADIR/test/tmpp2.txt;
>
>
> +--echo #
> +--echo # Bug#11765141 - 58072: LOAD DATA INFILE: LEAKS IO CACHE MEMORY
> +--echo # WHEN ERROR OCCURS
> +--echo #
> +
> +--let $file=$MYSQLTEST_VARDIR/tmp/bug11735141.txt
> +--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +--eval SELECT '1\n' INTO DUMPFILE '$file'
> +
> +create table t1(a point);
> +--replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +--error ER_CANT_CREATE_GEOMETRY_OBJECT
> +--eval LOAD DATA INFILE '$file' INTO TABLE t1
> +drop table t1;
> +
>   --echo End of 5.1 tests
>
> === modified file 'sql/sql_load.cc'
> --- a/sql/sql_load.cc	2010-12-17 13:05:50 +0000
> +++ b/sql/sql_load.cc	2011-04-07 13:09:19 +0000
> @@ -1075,9 +1075,10 @@ READ_INFO::READ_INFO(File file_par, uint
>   		     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),escape_char(escape)
> +  :file(file_par), buff_length(tot_length), escape_char(escape),
> +   found_end_of_line(false), eof(false), need_end_io_cache(false),
> +   error(false), line_cuted(false), found_null(false), read_charset(cs)
>   {
> -  read_charset= cs;
>     field_term_ptr=(char*) field_term.ptr();
>     field_term_length= field_term.length();
>     line_term_ptr=(char*) line_term.ptr();
> @@ -1104,8 +1105,6 @@ READ_INFO::READ_INFO(File file_par, uint
>       (uchar) enclosed_par[0] : INT_MAX;
>     field_term_char= field_term_length ? (uchar) field_term_ptr[0] : INT_MAX;
>     line_term_char= line_term_length ? (uchar) line_term_ptr[0] : INT_MAX;
> -  error=eof=found_end_of_line=found_null=line_cuted=0;
> -  buff_length=tot_length;
>
>
>     /* Set of a stack for unget if long terminators */
> @@ -1151,7 +1150,7 @@ READ_INFO::READ_INFO(File file_par, uint
>
>   READ_INFO::~READ_INFO()
>   {
> -  if (!error&&  need_end_io_cache)
> +  if (need_end_io_cache)
>       ::end_io_cache(&cache);
>
>     my_free(buffer, MYF(MY_ALLOW_ZERO_PTR));
>
>
>
>


--------------080405060707040700030609--