From: Davi Arnaut
Date: August 24 2010 12:40pm
Subject: Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3470)
 Bug#29751
List-Archive: http://lists.mysql.com/commits/116620
Message-Id: <4C73BDBC.8080800@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Dmitry,

On 8/10/10 4:02 AM, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug29751/ based on revid:davi.arnaut@stripped
>
>   3470 Dmitry Shulga	2010-08-10
>        Fixed bug #29751 - do not rename the error log at FLUSH LOGS.
>        Added open log file with FILE_SHARE_DELETE flag on Windows.
>       @ sql/log.cc
>          added reopen_fstreams();
>          modified redirect_std_streams(): call to sequence of freopen()
>          replaced to reopen_fstreams();
>          modified flush_error_log(): removed file rename for flushed
>          error log file.
>       @ sql/mysqld.cc
>          modified main() and init_server_components(): do open log error file
>          over call to reopen_fstreams().

Patch approved, but some comments below.

>      modified:
>        sql/log.cc
>        sql/mysqld.cc
> === modified file 'sql/log.cc'
> --- a/sql/log.cc	2010-07-20 18:07:36 +0000
> +++ b/sql/log.cc	2010-08-10 07:02:17 +0000
> @@ -5050,70 +5050,94 @@ void sql_perror(const char *message)
>   }
>
>
> +#ifdef __WIN__
> +extern "C" my_bool reopen_fstreams(const char * filename, FILE *outstream, FILE *errstream)

No space after the *.

> +{
> +  int handle_fd;                          /* handle of opened file */
> +  int stream_fd;
> +  HANDLE osfh;                            /* OS handle of opened file */
> +
> +  if (!filename || (!outstream && !errstream))
> +  {
> +    errno= EINVAL;
> +    return TRUE;

No need to return a error that the callers won't act on.
I suggest a debug assert instead.

[..]

> +#else
> +extern "C" my_bool reopen_fstreams(const char * filename, FILE *outstream, FILE *errstream)
> +{
> +  if (outstream && !freopen(filename,"a+",outstream))
> +    return TRUE;

Add space after the comma.

> +  if (errstream && !freopen(filename,"a+",errstream))
> +    return TRUE;

Same.

Regards,

Davi