List:Commits« Previous MessageNext Message »
From:Sergey Vojtovich Date:September 29 2010 11:23am
Subject:Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)
Bug#29071
View as plain text  
Hi Kristofer,

I have strong doubts about this patch.

First of all I tend to think that the true problem is not around
my_strdup(filename), but rather with inconsistency in my_close():
it is expected that my_close() is always called with valid file
descriptor, thus my_file_opened is decremented unconditionally.
But I have strong feeling that this requirement is not met, and
it is one more problem as such.

Here are a few relevant memories that came to my mind: probably
there was an assertion that my_close() gets valid file descriptor;
probably there was a bug in replication, which used to close random
file descriptors; probably there was a bug in myisam, which used to
call my_close(-1).


Secondly, my_file_opened/my_stream_opened were bound to physically
open files/streams. With this patch these counters are bound to files
which are successfully "registered" in my_file_info[] array. This is
probably a noble action, but I see no good reasons. We can fix (not
workaround, but truely fix) the problem with much lesser effort.


Thirdly, though open/close functionality seem to be trivial, it has
rather a tricky logics - lots of things needs to be taken into account.
I can already see a couple of problems with modified code, which makes me
worried. This code is basis for many things and is around for ages. I'd
prefer to avoid major modifications here, especially in GA.


For this particular bug I'd prefer to see a one-line fix in my_close().
I'm ok if you want to fix my_strdup() issue as well, but this is yet
another one-line fixes.

Regards,
Sergey

On Mon, Sep 27, 2010 at 01:11:50PM +0000, Kristofer Pettersson wrote:
> #At file:///home/thek/bzr/mysql-trunk/ based on
> revid:dlenev@stripped
> 
>  3197 Kristofer Pettersson	2010-09-27
>       Bug#29071 invalid Open_files or Slave_open_temp_tables value in show global
> status
>       
>       If an error occurred during registration of
>       a file handle, the Open_files counter would
>       show very large numbers.
>       
>       Only decrese my_file_opened or my_stream_opened if the file
>       descriptor was maked as anything but UNOPEN
>      @ mysys/my_fopen.c
>         * The variable "my_stream_opened" needs to be strongly associated
>           with the my_file_info::type attribute.
>      @ mysys/my_open.c
>         * The variable "my_file_opened" needs to be strongly associated
>           with the my_file_info::type attribute.
>      @ unittest/gunit/CMakeLists.txt
>         * Added two test cases my_open-t.cc, my_fopen-t.cc
> 
>     added:
>       unittest/gunit/my_fopen-t.cc
>       unittest/gunit/my_open-t.cc
>     modified:
>       mysys/my_fopen.c
>       mysys/my_open.c
>       unittest/gunit/CMakeLists.txt
> === modified file 'mysys/my_fopen.c'
> --- a/mysys/my_fopen.c	2010-07-08 21:20:08 +0000
> +++ b/mysys/my_fopen.c	2010-09-27 13:11:49 +0000
> @@ -20,18 +20,18 @@
>  
>  static void make_ftype(char * to,int flag);
>  
> -/*
> +/**
>    Open a file as stream
>  
> -  SYNOPSIS
> -    my_fopen()
> -    FileName	Path-name of file
> -    Flags	Read | write | append | trunc (like for open())
> -    MyFlags	Flags for handling errors
> -
> -  RETURN
> -    0	Error
> -    #	File handler
> +  @param FileName Path-name of file
> +  @param Flags like for open()
> +  @param MyFlags Flags for handling errors
> +
> +  @see my_win_fopen()
> +  
> +  @return
> +    @retval 0 Error
> +    @retval # File handler
>  */
>  
>  FILE *my_fopen(const char *filename, int flags, myf MyFlags)
> @@ -52,20 +52,27 @@ FILE *my_fopen(const char *filename, int
>    if (fd != 0)
>    {
>      /*
> -      The test works if MY_NFILE < 128. The problem is that fileno() is char
> -      on some OS (SUNOS). Actually the filename save isn't that important
> -      so we can ignore if this doesn't work.
> +      @todo The problem is that fileno() is of type char on some OS (SUNOS).
>      */
>  
>      int filedesc= my_fileno(fd);
> +    mysql_mutex_lock(&THR_LOCK_open); 
>      if ((uint)filedesc >= my_file_limit)
>      {
> -      thread_safe_increment(my_stream_opened,&THR_LOCK_open);
> -      DBUG_RETURN(fd);				/* safeguard */
> +      /*
> +        The number of concurrently opened files exceeds the limits or
> +        the file descriptor can't be mapped to the my_file_info array.
> +      */
> +      my_errno= EMFILE;
> +#ifndef _WIN32
> +      fclose(fd);
> +#else
> +      my_win_fclose(fd);
> +#endif
> +      mysql_mutex_unlock(&THR_LOCK_open);
> +      goto err;
>      }
> -    mysql_mutex_lock(&THR_LOCK_open);
> -    if ((my_file_info[filedesc].name= (char*)
> -	 my_strdup(filename,MyFlags)))
> +    if ((my_file_info[filedesc].name= (char*) my_strdup(filename,MyFlags)))
>      {
>        my_stream_opened++;
>        my_file_total_opened++;
> @@ -80,6 +87,8 @@ FILE *my_fopen(const char *filename, int
>    }
>    else
>      my_errno=errno;
> +    
> +err:
>    DBUG_PRINT("error",("Got error %d on open",my_errno));
>    if (MyFlags & (MY_FFNF | MY_FAE | MY_WME))
>      my_error((flags & O_RDONLY) || (flags == O_RDONLY ) ? EE_FILENOTFOUND :
> @@ -98,13 +107,25 @@ int my_fclose(FILE *fd, myf MyFlags)
>    DBUG_ENTER("my_fclose");
>    DBUG_PRINT("my",("stream: 0x%lx  MyFlags: %d", (long) fd, MyFlags));
>  
> -  mysql_mutex_lock(&THR_LOCK_open);
> +  if(fd == 0)
> +  {
> +    /* Function called with wrong parameters */
> +    DBUG_RETURN(EBADF);
> +  }
> +  
>    file= my_fileno(fd);
> +  if ((uint) file >= my_file_limit)
> +  {
> +    /* Function called with wrong parameters */
> +    DBUG_RETURN(EBADF);
> +  }
> +  
>  #ifndef _WIN32
>    err= fclose(fd);
>  #else
>    err= my_win_fclose(fd);
>  #endif
> +  mysql_mutex_lock(&THR_LOCK_open);
>    if(err < 0)
>    {
>      my_errno=errno;
> @@ -112,12 +133,12 @@ int my_fclose(FILE *fd, myf MyFlags)
>        my_error(EE_BADCLOSE, MYF(ME_BELL+ME_WAITTANG),
>  	       my_filename(file),errno);
>    }
> -  else
> -    my_stream_opened--;
> -  if ((uint) file < my_file_limit && my_file_info[file].type != UNOPEN)
> +
> +  if (my_file_info[file].type != UNOPEN)
>    {
>      my_file_info[file].type = UNOPEN;
>      my_free(my_file_info[file].name);
> +    my_stream_opened--;
>    }
>    mysql_mutex_unlock(&THR_LOCK_open);
>    DBUG_RETURN(err);
> 
> === modified file 'mysys/my_open.c'
> --- a/mysys/my_open.c	2010-07-08 21:20:08 +0000
> +++ b/mysys/my_open.c	2010-09-27 13:11:49 +0000
> @@ -19,17 +19,20 @@
>  #include <errno.h>
>  
>  
> -/*
> +/**
>    Open a file
>  
> -  SYNOPSIS
> -    my_open()
> -      FileName	Fully qualified file name
> -      Flags	Read | write 
> -      MyFlags	Special flags
> -
> -  RETURN VALUE
> -    File descriptor
> +  @param FileName Fully qualified file name
> +  @param Flags Same as for unix system call open(2)
> +  @param MyFlags Special flags
> +
> +  @note This function encapsluates my_win_open and open(3) calls.
> +  
> +  @see my_win_open
> + 
> +  @return
> +    @retval -1 An error occurred; my_errno has the error code.
> +    @retval #  On success a file descriptor is returned.
>  */
>  
>  File my_open(const char *FileName, int Flags, myf MyFlags)
> @@ -70,6 +73,11 @@ int my_close(File fd, myf MyFlags)
>    DBUG_ENTER("my_close");
>    DBUG_PRINT("my",("fd: %d  MyFlags: %d",fd, MyFlags));
>  
> +  if (fd < 0 || fd >= my_file_limit)
> +  {
> +    /* Function called with wrong parameters */
> +    DBUG_RETURN(EBADF);  
> +  }
>    mysql_mutex_lock(&THR_LOCK_open);
>  #ifndef _WIN32
>    do
> @@ -86,15 +94,15 @@ int my_close(File fd, myf MyFlags)
>      if (MyFlags & (MY_FAE | MY_WME))
>        my_error(EE_BADCLOSE, MYF(ME_BELL+ME_WAITTANG),my_filename(fd),errno);
>    }
> -  if ((uint) fd < my_file_limit && my_file_info[fd].type != UNOPEN)
> +  if (my_file_info[fd].type != UNOPEN)
>    {
>      my_free(my_file_info[fd].name);
>  #if defined(THREAD) && !defined(HAVE_PREAD) && !defined(_WIN32)
>      mysql_mutex_destroy(&my_file_info[fd].mutex);
>  #endif
>      my_file_info[fd].type = UNOPEN;
> +    my_file_opened--;
>    }
> -  my_file_opened--;
>    mysql_mutex_unlock(&THR_LOCK_open);
>    DBUG_RETURN(err);
>  } /* my_close */
> @@ -103,17 +111,15 @@ int my_close(File fd, myf MyFlags)
>  /*
>    Register file in my_file_info[]
>     
> -  SYNOPSIS
> -    my_register_filename()
> -    fd			   File number opened, -1 if error on open
> -    FileName		   File name
> -    type_file_type	   How file was created
> -    error_message_number   Error message number if caller got error (fd == -1)
> -    MyFlags		   Flags for my_close()
> -
> -  RETURN
> -    -1   error
> -     #   Filenumber
> +  @param fd File number opened, -1 if error on open
> +  @param FileName File name
> +  @param type_file_type How file was created
> +  @param error_message_numbe Error message number if caller got error (fd == -1)
> +  @param MyFlags Flags for my_close()
> +
> +  @return
> +    @retval -1 An error occurred
> +    @retval # On success the file descriptor is returned.
>  
>  */
>  
> @@ -121,41 +127,56 @@ File my_register_filename(File fd, const
>  			  type_of_file, uint error_message_number, myf MyFlags)
>  {
>    DBUG_ENTER("my_register_filename");
> -  if ((int) fd >= MY_FILE_MIN)
> +  if (fd < 0)
>    {
> -    if ((uint) fd >= my_file_limit)
> +    /*
> +      The file descriptor indicates that the FileName couldn't be opened
> +      properly; - Report the error.
> +    */
> +    my_errno= errno;
> +    goto err;
> +  }
> + 
> +  mysql_mutex_lock(&THR_LOCK_open); 
> +  if (fd >= my_file_limit)
> +  {
> +    /*
> +      The number of concurrently opened files exceeds the limits or
> +      the file descriptor can't be mapped to the my_file_info array.
> +    */
> +    my_errno= EMFILE;
> +#ifndef _WIN32
> +    int ret;
> +    do
>      {
> -#if defined(THREAD) && !defined(HAVE_PREAD) 
> -      my_errno= EMFILE;
> +      ret= close(fd);
> +    } while (ret == -1 && errno == EINTR);
>  #else
> -      thread_safe_increment(my_file_opened,&THR_LOCK_open);
> -      DBUG_RETURN(fd);				/* safeguard */
> -#endif
> -    }
> -    else
> -    {
> -      mysql_mutex_lock(&THR_LOCK_open);
> -      if ((my_file_info[fd].name = (char*) my_strdup(FileName,MyFlags)))
> -      {
> -        my_file_opened++;
> -        my_file_total_opened++;
> -        my_file_info[fd].type = type_of_file;
> -#if defined(THREAD) && !defined(HAVE_PREAD) && !defined(_WIN32)
> -        mysql_mutex_init(key_my_file_info_mutex, &my_file_info[fd].mutex,
> -                         MY_MUTEX_INIT_FAST);
> +    my_win_close(fd);
>  #endif
> -        mysql_mutex_unlock(&THR_LOCK_open);
> -        DBUG_PRINT("exit",("fd: %d",fd));
> -        DBUG_RETURN(fd);
> -      }
> -      mysql_mutex_unlock(&THR_LOCK_open);
> -      my_errno= ENOMEM;
> -    }
> -    (void) my_close(fd, MyFlags);
> +    mysql_mutex_unlock(&THR_LOCK_open);
> +    goto err;
>    }
> -  else
> -    my_errno= errno;
>  
> +  if ((my_file_info[fd].name = (char*) my_strdup(FileName,MyFlags)))
> +  {
> +    my_file_opened++;
> +    my_file_total_opened++;
> +    my_file_info[fd].type = type_of_file;
> +#if defined(THREAD) && !defined(_WIN32) && !defined(HAVE_PREAD)
> +    mysql_mutex_init(key_my_file_info_mutex, &my_file_info[fd].mutex,
> +                     MY_MUTEX_INIT_FAST);
> +#endif
> +    mysql_mutex_unlock(&THR_LOCK_open);
> +    DBUG_PRINT("exit",("fd: %d",fd));
> +    DBUG_RETURN(fd);
> +  }
> +#if defined(THREAD)
> +  mysql_mutex_unlock(&THR_LOCK_open);
> +#endif
> +  my_errno= ENOMEM;
> +
> +err:
>    DBUG_PRINT("error",("Got error %d on open", my_errno));
>    if (MyFlags & (MY_FFNF | MY_FAE | MY_WME))
>    {
> 
> === modified file 'unittest/gunit/CMakeLists.txt'
> --- a/unittest/gunit/CMakeLists.txt	2010-07-30 08:34:23 +0000
> +++ b/unittest/gunit/CMakeLists.txt	2010-09-27 13:11:49 +0000
> @@ -207,7 +207,7 @@ IF (CMAKE_CXX_COMPILER_ID STREQUAL "SunP
>  ENDIF()
>  
>  # Add tests (link them with sql library) 
> -SET(TESTS sql_list mdl mdl_mytap my_regex thread_utils)
> +SET(TESTS sql_list mdl mdl_mytap my_regex thread_utils my_open my_fopen)
>  FOREACH(test ${TESTS})
>    ADD_EXECUTABLE(${test}-t ${test}-t.cc)
>    TARGET_LINK_LIBRARIES(${test}-t gunit sqlgunitlib strings dbug regex)
> 
> === added file 'unittest/gunit/my_fopen-t.cc'
> --- a/unittest/gunit/my_fopen-t.cc	1970-01-01 00:00:00 +0000
> +++ b/unittest/gunit/my_fopen-t.cc	2010-09-27 13:11:49 +0000
> @@ -0,0 +1,191 @@
> +/* Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; version 2 of the License.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software Foundation,
> +   51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA */
> +
> +#include "my_config.h"
> +#include <gtest/gtest.h>
> +#include "mysqld_error.h"
> +#include "my_sys.h"
> +
> +#define MAX_NUMBER_OF_FILE_DESCRIPTORS (OS_FILE_LIMIT+MY_NFILE+1024)
> +#define DUMMY_FILE_NAME "successful_open_rdonly.bin"
> +
> +struct st_my_file_info my_file_info_default[MY_NFILE];
> +class My_open_test : public ::testing::Test
> +{
> +public:
> +  My_open_test()
> +  {
> +  }
> +  
> +  virtual void SetUp()
> +  {
> +    /* set all global variables for each TEST_F. */
> +    my_thread_basic_global_reinit();
> +    my_stream_opened= 0;
> +    my_file_total_opened= 0;
> +    my_set_max_open_files(MAX_NUMBER_OF_FILE_DESCRIPTORS);
> +    my_errno= 0;
> +  }
> +  
> +  /**
> +    Set up test case for all tests
> +  */
> +  static void SetUpTestCase()
> +  {
> +    my_thread_basic_global_init();
> +    FILE *fp;
> +    fp= my_fopen(DUMMY_FILE_NAME, O_WRONLY | O_CREAT | O_BINARY, MYF(0));
> +    if (fp == 0)
> +    {
> +      ADD_FAILURE() << "Failed to SetUpTestCase().";
> +      exit(EXIT_FAILURE);
> +    }
> +    my_fclose(fp, MYF(0));
> +  }
> +  
> +  static void TearDownTestCase()
> +  {
> +    /*
> +      The std lib routine unlink(3) is considered trusted.
> +    */
> +    if (unlink(DUMMY_FILE_NAME) > 0)
> +    {
> +      ADD_FAILURE() << "Failed to TearDownTestCase().";
> +      exit(EXIT_FAILURE);
> +    }
> +  }
> +
> +private:
> +
> +};
> +
> +TEST_F(My_open_test, EnvironmentCheck)
> +{
> +}
> +
> +TEST_F(My_open_test, FailToOpenUnknownFile)
> +{
> +  FILE *fp;
> +  fp= my_fopen("/really/unknown/does-not-exist.bin", O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_EQ((FILE *)0, fp);
> +  EXPECT_EQ(0, my_stream_opened);
> +  EXPECT_EQ(0, my_file_total_opened);
> +}
> +
> +
> +TEST_F(My_open_test, SuccessToOpenKnownBinaryFile)
> +{
> +  EXPECT_EQ(0, my_stream_opened);
> +  FILE *fp;
> +  fp= my_fopen(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +  /*
> +    The file should exist in the test dir and the call to my_open should not
> +    fail under normal circumstances.
> +  */
> +  EXPECT_NE((FILE *)0, fp);
> +  
> +  /*
> +    The number of currently opened files should have increased
> +  */
> +  EXPECT_EQ(1, my_stream_opened);
> +  /*
> +    The number of total files should have increased
> +  */
> +  EXPECT_EQ(1, my_file_total_opened);
> +  
> +  
> +  /*
> +    When the file is closed the number of currently opened files should have
> +    decreased to 0.
> +  */
> +  my_fclose(fp, MYF(0));
> +  EXPECT_EQ(0, my_stream_opened);  
> +
> +  /*
> +    But the number of files which has been opened should of course stay the
> +    same.
> +  */
> +  EXPECT_EQ(1, my_file_total_opened);
> +  
> +  /*
> +    Lets try it again to see if the number of files ever opened in the system
> +    increases to 2
> +  */
> +  fp= my_fopen(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));  
> +  EXPECT_NE((FILE *)0, fp);
> +  EXPECT_EQ(1, my_stream_opened);
> +  EXPECT_EQ(2, my_file_total_opened);
> +  my_fclose(fp, MYF(0));
> +  EXPECT_EQ(0, my_stream_opened);  
> +  EXPECT_EQ(2, my_file_total_opened);
> +  
> +}
> +
> +TEST_F(My_open_test, TooManyOpenFiles)
> +{
> +  FILE *fp[MAX_NUMBER_OF_FILE_DESCRIPTORS+1];
> +  /*
> +    Attempt to open MAX_NUMBER_OF_FILE_DESCRIPTORS number of files.
> +    NOTE: Couting begins at 0 hence we exclude i==MAX_NUMBER_OF_FILE_DESCRIPTORS
> +  */
> +  int opened_files= 0;
> +  for(unsigned i= 0; i<MAX_NUMBER_OF_FILE_DESCRIPTORS; ++i)
> +  {
> +    fp[i]= my_fopen(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +    if (fp[i] == 0)
> +      break;
> +    else
> +      ++opened_files;
> +  }
> +  /*
> +    We now have depleted the number of available file descriptors.
> +    Next attempt to open a file will fail.
> +  */
> +  fp[opened_files]= my_fopen(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_EQ((FILE *)0, fp[opened_files]);
> +
> +  /*
> +    Apply my_fclose) on all file descriptors to make sure all files are closed
> +  */
> +  for(int i= opened_files; i>=0; --i)
> +  {
> +    my_fclose(fp[i],MYF(0));
> +  }
> +  /*
> +    No more files should be opened.
> +  */
> +  EXPECT_EQ(0, my_stream_opened);
> +  EXPECT_EQ(opened_files, my_file_total_opened);
> +}
> +
> +TEST_F(My_open_test, MultipleClose)
> +{
> +  FILE *fp;
> +  fp= my_fopen(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_NE((FILE *)0, fp);
> +  my_fclose(fp, MYF(0));
> +  my_fclose(fp, MYF(0));
> +  my_fclose(fp, MYF(0));
> +  EXPECT_EQ(0, my_stream_opened);  
> +  EXPECT_EQ(1, my_file_total_opened);
> +  fp= my_fopen("/really/dont/exist.bin", O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_EQ((FILE *)0, fp);
> +  my_fclose(fp, MYF(0));
> +  my_fclose(fp, MYF(0));
> +  my_fclose(fp, MYF(0));
> +  EXPECT_EQ(0, my_stream_opened);  
> +  EXPECT_EQ(1, my_file_total_opened);
> +}
> +
> 
> === added file 'unittest/gunit/my_open-t.cc'
> --- a/unittest/gunit/my_open-t.cc	1970-01-01 00:00:00 +0000
> +++ b/unittest/gunit/my_open-t.cc	2010-09-27 13:11:49 +0000
> @@ -0,0 +1,182 @@
> +/* Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; version 2 of the License.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program; if not, write to the Free Software Foundation,
> +   51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA */
> +
> +#include "my_config.h"
> +#include <gtest/gtest.h>
> +#include "mysqld_error.h"
> +#include "my_sys.h"
> +
> +#define MAX_NUMBER_OF_FILE_DESCRIPTORS (OS_FILE_LIMIT+MY_NFILE+1024)
> +#define DUMMY_FILE_NAME "successful_open_rdonly.bin"
> +
> +struct st_my_file_info my_file_info_default[MY_NFILE];
> +class My_open_test : public ::testing::Test
> +{
> +public:
> +  My_open_test()
> +  {
> +  }
> +  
> +  virtual void SetUp()
> +  {
> +    /* set all global variables for each TEST_F */
> +    my_thread_basic_global_reinit();
> +    my_file_opened= 0;
> +    my_file_total_opened= 0;
> +    my_set_max_open_files(MAX_NUMBER_OF_FILE_DESCRIPTORS);
> +    my_errno= 0;
> +  }
> +  
> +  static void SetUpTestCase()
> +  {
> +    my_thread_basic_global_init();
> +    File fd;
> +    fd= my_open(DUMMY_FILE_NAME, O_WRONLY | O_CREAT | O_BINARY, MYF(0));
> +    if (fd == -1)
> +    {
> +      ADD_FAILURE() << "Failed to SetUpTestCase().";
> +      exit(EXIT_FAILURE);
> +    }
> +    my_close(fd, MYF(0));
> +  }
> +  
> +  static void TearDownTestCase()
> +  {
> +    /*
> +      The std lib routine unlink(3) is considered trusted.
> +    */
> +    if (unlink(DUMMY_FILE_NAME) > 0)
> +    {
> +      ADD_FAILURE() << "Failed to TearDownTestCase().";
> +      exit(EXIT_FAILURE);
> +    }
> +  }
> +
> +private:
> +
> +};
> +
> +TEST_F(My_open_test, FailToOpenUnknownFile)
> +{
> +  File fd;
> +  fd= my_open("/really/unknown/does-not-exist.bin", O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_EQ(-1, fd);
> +  EXPECT_EQ(0, my_file_opened);
> +  EXPECT_EQ(0, my_file_total_opened);
> +}
> +
> +
> +TEST_F(My_open_test, SuccessToOpenKnownBinaryFile)
> +{
> +  File fd;
> +  fd= my_open(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +  /*
> +    The file should exist in the test dir and the call to my_open should not
> +    fail under normal circumstances.
> +  */
> +  EXPECT_NE(-1, fd);
> +  
> +  /*
> +    The number of currently opened files should have increased
> +  */
> +  EXPECT_EQ(1, my_file_opened);
> +  /*
> +    The number of total files should have increased
> +  */
> +  EXPECT_EQ(1, my_file_total_opened);
> +  
> +  my_close(fd, MYF(0));
> +  
> +  /*
> +    When the file is closed the number of currently opened files should have
> +    decreased to 0.
> +  */
> +  EXPECT_EQ(0, my_file_opened);  
> +  /*
> +    But the number of files which has been opened should of course stay the
> +    same.
> +  */
> +  EXPECT_EQ(1, my_file_total_opened);
> +  
> +  /*
> +    Lets try it again to see if the number of files ever opened in the system
> +    increases to 2
> +  */
> +  fd= my_open(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));  
> +  EXPECT_NE(-1, fd);
> +  EXPECT_EQ(1, my_file_opened);
> +  EXPECT_EQ(2, my_file_total_opened);
> +  my_close(fd, MYF(0));
> +  EXPECT_EQ(0, my_file_opened);  
> +  EXPECT_EQ(2, my_file_total_opened);
> +  
> +}
> +
> +TEST_F(My_open_test, TooManyOpenFiles)
> +{
> +  File fd[MAX_NUMBER_OF_FILE_DESCRIPTORS+1];
> +  /*
> +    Attempt to open MAX_NUMBER_OF_FILE_DESCRIPTORS number of files.
> +    NOTE: Couting begins at 0 hence we exclude i==MAX_NUMBER_OF_FILE_DESCRIPTORS
> +  */
> +  int opened_files= 0;
> +  for(unsigned i= 0; i<MAX_NUMBER_OF_FILE_DESCRIPTORS; ++i)
> +  {
> +    fd[i]= my_open(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +    if (fd[i] == -1)
> +      break;
> +    else
> +      ++opened_files;
> +  }
> +  /*
> +    We now have depleted the number of available file descriptors.
> +    Next attempt to open a file will fail.
> +  */
> +  fd[opened_files]= my_open(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_EQ(-1, fd[opened_files]);
> +
> +  /*
> +    Apply my_close() on all file descriptors to make sure all files are closed
> +  */
> +  for(int i= opened_files; i>=0; --i)
> +  {
> +    my_close(fd[i],MYF(0));
> +  }
> +  /*
> +    No more files should be opened.
> +  */
> +  EXPECT_EQ(0, my_file_opened);
> +  EXPECT_EQ(opened_files, my_file_total_opened);
> +}
> +
> +TEST_F(My_open_test, MultipleClose)
> +{
> +  File fd;
> +  fd= my_open(DUMMY_FILE_NAME, O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_NE(-1, fd);
> +  my_close(fd, MYF(0));
> +  my_close(fd, MYF(0));
> +  my_close(fd, MYF(0));
> +  EXPECT_EQ(0, my_file_opened);  
> +  EXPECT_EQ(1, my_file_total_opened);
> +  fd= my_open("/really/dont/exist.bin", O_RDONLY | O_BINARY, MYF(0));
> +  EXPECT_EQ(-1, fd);
> +  my_close(fd, MYF(0));
> +  my_close(fd, MYF(0));
> +  my_close(fd, MYF(0));
> +  EXPECT_EQ(0, my_file_opened);  
> +  EXPECT_EQ(1, my_file_total_opened);
> +}
> +
> 


> 
> -- 
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    http://lists.mysql.com/commits?unsub=1


-- 
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
Thread
bzr commit into mysql-trunk branch (kristofer.pettersson:3197) Bug#29071Kristofer Pettersson27 Sep
Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Kristofer Pettersson29 Sep
  • Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Sergey Vojtovich30 Sep
    • Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Kristofer Pettersson30 Sep
      • Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Sergey Vojtovich1 Oct
Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Sergey Vojtovich29 Sep
Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Kristofer Pettersson1 Oct
  • Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Sergey Vojtovich1 Oct
    • Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Kristofer Pettersson1 Oct
      • Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Sergey Vojtovich1 Oct
        • Re: bzr commit into mysql-trunk branch (kristofer.pettersson:3197)Bug#29071Kristofer Pettersson4 Oct