List:Commits« Previous MessageNext Message »
From:kpettersson Date:October 31 2006 8:26am
Subject:bk commit into 4.1 tree (thek:1.2543) BUG#22828
View as plain text  
Below is the list of changes that have just been committed into a local
4.1 repository of thek. When thek does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2006-10-31 09:26:16+01:00, thek@stripped +4 -0
  Bug#22828 _my_b_read() ignores return values for my_seek() calls
  - Because my_seek actually is capable of returning an error code we should
    exploit that in the best possible way.
  - There might be kernel errors or other errors we can't predict and capturing
    the return value of all system calls gives us better understanding of
    possible errors.

  mysys/mf_iocache.c@stripped, 2006-10-31 09:26:15+01:00, thek@stripped +90 -12
    - Added check on return value for my_seek
    - Added comments

  mysys/my_chsize.c@stripped, 2006-10-31 09:26:15+01:00, thek@stripped +5 -1
    - Added check on return value for my_seek
    - Added comments

  mysys/my_lock.c@stripped, 2006-10-31 09:26:15+01:00, thek@stripped +34 -5
    - Added check on return value for my_seek
    - Added comments

  mysys/my_seek.c@stripped, 2006-10-31 09:26:15+01:00, thek@stripped +24 -2
    - Added comments

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	thek
# Host:	kpdesk.mysql.com
# Root:	/home/thek/dev/bug22828/my41-bug22828

--- 1.48/mysys/mf_iocache.c	2006-10-31 09:26:20 +01:00
+++ 1.49/mysys/mf_iocache.c	2006-10-31 09:26:20 +01:00
@@ -440,11 +440,24 @@ int _my_b_read(register IO_CACHE *info, 
 
   /* pos_in_file always point on where info->buffer was read */
   pos_in_file=info->pos_in_file+(uint) (info->read_end - info->buffer);
+
+  /* 
+    Whenever a function which operates on IO_CACHE flushes/writes
+    some part of the IO_CACHE to disk it will set the property
+    "seek_not_done" to indicate this to other functions operating
+    on the IO_CACHE.
+  */
   if (info->seek_not_done)
-  {					/* File touched, do seek */
-    VOID(my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0)));
+  {
+    if (my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0)) 
+        == MY_FILEPOS_ERROR)
+    {
+        info->error= -1;
+        DBUG_RETURN(1);
+    }
     info->seek_not_done=0;
   }
+
   diff_length=(uint) (pos_in_file & (IO_SIZE-1));
   if (Count >= (uint) (IO_SIZE+(IO_SIZE-diff_length)))
   {					/* Fill first intern buffer */
@@ -633,8 +646,22 @@ int _my_b_read_r(register IO_CACHE *info
     if (lock_io_cache(info, pos_in_file))
     {
       info->share->active=info;
-      if (info->seek_not_done)             /* File touched, do seek */
-        VOID(my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0)));
+      /*
+        Whenever a function which operates on IO_CACHE flushes/writes
+        some part of the IO_CACHE to disk it will set the property
+        "seek_not_done" to indicate this to other functions operating
+        on the IO_CACHE.
+      */
+      if (info->seek_not_done)
+      {
+        if (my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0))
+            == MY_FILEPOS_ERROR)
+        {
+          info->error= -1;
+          unlock_io_cache(info);
+          DBUG_RETURN(1);
+        }
+      }
       len=(int)my_read(info->file,info->buffer, length, info->myflags);
       info->read_end=info->buffer + (len == -1 ? 0 : len);
       info->error=(len == (int)length ? 0 : len);
@@ -668,11 +695,16 @@ int _my_b_read_r(register IO_CACHE *info
 
 
 /*
-  Do sequential read from the SEQ_READ_APPEND cache
-  we do this in three stages:
+  Do sequential read from the SEQ_READ_APPEND cache.
+  
+  We do this in three stages:
    - first read from info->buffer
    - then if there are still data to read, try the file descriptor
    - afterwards, if there are still data to read, try append buffer
+
+  RETURNS
+    0  Success
+    1  Failed to read
 */
 
 int _my_b_seq_read(register IO_CACHE *info, byte *Buffer, uint Count)
@@ -700,7 +732,13 @@ int _my_b_seq_read(register IO_CACHE *in
     With read-append cache we must always do a seek before we read,
     because the write could have moved the file pointer astray
   */
-  VOID(my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0)));
+  if (my_seek(info->file,pos_in_file,MY_SEEK_SET,MYF(0))
+      == MY_FILEPOS_ERROR)
+  {
+   info->error= -1;
+   unlock_append_buffer(info);
+   return (1);
+  }
   info->seek_not_done=0;
 
   diff_length=(uint) (pos_in_file & (IO_SIZE-1));
@@ -816,6 +854,21 @@ read_append_buffer:
 
 #ifdef HAVE_AIOWAIT
 
+/*
+  Read from the IO_CACHE into a buffer and feed asynchronously
+  from disk when needed.
+
+  SYNOPSIS
+    _my_b_async_read()
+      info                      IO_CACHE pointer
+      Buffer                    Buffer to retrieve count bytes from file
+      Count                     Number of bytes to read into Buffer
+
+  RETURN VALUE
+    -1          An error has occurred; my_errno is set.
+     0          Success
+     1          An error has occurred; IO_CACHE to error state.
+*/
 int _my_b_async_read(register IO_CACHE *info, byte *Buffer, uint Count)
 {
   uint length,read_length,diff_length,left_length,use_length,org_Count;
@@ -906,13 +959,20 @@ int _my_b_async_read(register IO_CACHE *
       info->error=(int) (read_length+left_length);
       return 1;
     }
-    VOID(my_seek(info->file,next_pos_in_file,MY_SEEK_SET,MYF(0)));
+    
+    if (my_seek(info->file,next_pos_in_file,MY_SEEK_SET,MYF(0))
+        == MY_FILEPOS_ERROR)
+    {
+      info->error= -1;
+      return (1);
+    }
+
     read_length=IO_SIZE*2- (uint) (next_pos_in_file & (IO_SIZE-1));
     if (Count < read_length)
     {					/* Small block, read to cache */
       if ((read_length=my_read(info->file,info->request_pos,
 			       read_length, info->myflags)) == (uint) -1)
-	return info->error= -1;
+        return info->error= -1;
       use_length=min(Count,read_length);
       memcpy(Buffer,info->request_pos,(size_t) use_length);
       info->read_pos=info->request_pos+Count;
@@ -999,7 +1059,15 @@ int _my_b_get(IO_CACHE *info)
   return (int) (uchar) buff;
 }
 
-	/* Returns != 0 if error on write */
+/* 
+   Write a byte buffer to IO_CACHE and flush to disk
+   if IO_CACHE is full.
+
+   RETURN VALUE
+    1 On error on write
+    0 On success
+   -1 On error; my_errno contains error code.
+*/
 
 int _my_b_write(register IO_CACHE *info, const byte *Buffer, uint Count)
 {
@@ -1022,8 +1090,18 @@ int _my_b_write(register IO_CACHE *info,
   {					/* Fill first intern buffer */
     length=Count & (uint) ~(IO_SIZE-1);
     if (info->seek_not_done)
-    {					/* File touched, do seek */
-      VOID(my_seek(info->file,info->pos_in_file,MY_SEEK_SET,MYF(0)));
+    {
+      /*
+        Whenever a function which operates on IO_CACHE flushes/writes
+        some part of the IO_CACHE to disk it will set the property
+        "seek_not_done" to indicate this to other functions operating
+        on the IO_CACHE.
+      */
+      if (my_seek(info->file,info->pos_in_file,MY_SEEK_SET,MYF(0)))
+      {
+        info->error= -1;
+        return (1);
+      }
       info->seek_not_done=0;
     }
     if (my_write(info->file,Buffer,(uint) length,info->myflags | MY_NABP))

--- 1.15/mysys/my_chsize.c	2006-10-31 09:26:20 +01:00
+++ 1.16/mysys/my_chsize.c	2006-10-31 09:26:20 +01:00
@@ -88,7 +88,11 @@ int my_chsize(File fd, my_off_t newlengt
       Fill space between requested length and true length with 'filler'
       We should never come here on any modern machine
     */
-    VOID(my_seek(fd, newlength, MY_SEEK_SET, MYF(MY_WME+MY_FAE)));
+    if (my_seek(fd, newlength, MY_SEEK_SET, MYF(MY_WME+MY_FAE))
+        == MY_FILEPOS_ERROR)
+    {
+      goto err;
+    }
     swap_variables(my_off_t, newlength, oldsize);
   }
 #endif

--- 1.12/mysys/my_lock.c	2006-10-31 09:26:20 +01:00
+++ 1.13/mysys/my_lock.c	2006-10-31 09:26:20 +01:00
@@ -35,7 +35,14 @@
 #include <nks/fsio.h>
 #endif
 
-	/* Lock a part of a file */
+/* 
+  Lock a part of a file 
+
+  RETURN VALUE
+    0   Success
+    -1  An error has occured and 'my_errno' is set
+        to indicate the actual error code.
+*/
 
 int my_lock(File fd, int locktype, my_off_t start, my_off_t length,
 	    myf MyFlags)
@@ -104,10 +111,22 @@ int my_lock(File fd, int locktype, my_of
 #elif defined(HAVE_LOCKING)
   /* Windows */
   {
-    my_bool error;
+    my_bool error= false;
     pthread_mutex_lock(&my_file_info[fd].mutex);
-    if (MyFlags & MY_SEEK_NOT_DONE)
-      VOID(my_seek(fd,start,MY_SEEK_SET,MYF(MyFlags & ~MY_SEEK_NOT_DONE)));
+    if (MyFlags & MY_SEEK_NOT_DONE) 
+    {
+      if( my_seek(fd,start,MY_SEEK_SET,MYF(MyFlags & ~MY_SEEK_NOT_DONE))
+           == MY_FILEPOS_ERROR )
+      {
+        /*
+          If my_seek fails my_errno will already contain an error code;
+          just unlock and return error code.
+         */
+        DBUG_PRINT("error",("my_errno: %d (%d)",my_errno,errno));
+        pthread_mutex_unlock(&my_file_info[fd].mutex);
+        DBUG_RETURN(-1);
+      }
+    }
     error= locking(fd,locktype,(ulong) length) && errno != EINVAL;
     pthread_mutex_unlock(&my_file_info[fd].mutex);
     if (!error)
@@ -145,7 +164,17 @@ int my_lock(File fd, int locktype, my_of
   }
 #else
   if (MyFlags & MY_SEEK_NOT_DONE)
-    VOID(my_seek(fd,start,MY_SEEK_SET,MYF(MyFlags & ~MY_SEEK_NOT_DONE)));
+  {
+    if (my_seek(fd,start,MY_SEEK_SET,MYF(MyFlags & ~MY_SEEK_NOT_DONE))
+        == MY_FILEPOS_ERROR)
+    {
+      /*
+        If an error has occured in my_seek then we will already
+        have an error code in my_errno; Just return error code.
+      */
+      DBUG_RETURN(-1);
+    }
+  }
   if (lockf(fd,locktype,length) != -1)
     DBUG_RETURN(0);
 #endif /* HAVE_FCNTL */

--- 1.11/mysys/my_seek.c	2006-10-31 09:26:20 +01:00
+++ 1.12/mysys/my_seek.c	2006-10-31 09:26:20 +01:00
@@ -16,8 +16,30 @@
 
 #include "mysys_priv.h"
 
-	/* Seek to position in file */
-	/*ARGSUSED*/
+/* 
+  Seek to a position in a file.
+
+  ARGUMENTS
+  File fd          The file descriptor
+  my_off_t pos     The expected position (absolute or relative)
+  int whence       A direction parameter and one of 
+                   {SEEK_SET, SEEK_CUR, SEEK_END}
+  myf MyFlags      Not used.
+
+  DESCRIPTION
+    The my_seek  function  is a wrapper around the system call lseek and
+    repositions  the  offset of the file descriptor fd to the argument 
+    offset according to the directive whence as follows:
+      SEEK_SET    The offset is set to offset bytes.
+      SEEK_CUR    The offset is set to its current location plus offset bytes
+      SEEK_END    The offset is set to the size of the file plus offset bytes
+
+  RETURN VALUE
+    my_off_t newpos    The new position in the file.
+    MY_FILEPOS_ERROR   An error was encountered while performing 
+                       the seek. my_errno is set to indicate the
+                       actual error.
+*/
 
 my_off_t my_seek(File fd, my_off_t pos, int whence,
 		 myf MyFlags __attribute__((unused)))
Thread
bk commit into 4.1 tree (thek:1.2543) BUG#22828kpettersson31 Oct