Subject: Re: cp problem on Win95 / lseek past EOF
From: [email-masked] (Vince Del Vecchio)

27 Jul 1997 13:31:14 -0700
Approved: [email-masked]
Distribution: cygnus
Message-ID: <[email-masked]>
Original-To: [email-masked]
Original-Sender: [email-masked]

After further investigation, I have several bugs to report.

1) In fileutils, ST_NBLOCKS in system.h is supposed to return the
   number of 512 byte blocks.  It Cygwin32, it seems that st_blksize is
   1024, but ST_NBLOCKS is still defined as simply the st_blocks field,
   so it is off by a factor of two.

   (This is why cp decides that files are sparse, and triggers bug #6.)

2) In diffutils, something should cause HAVE_SETMODE to be defined so
   that diff has the --binary option, and cmp works in binary mode.
   cmp doesn't work very well in text mode...  It is prone to dangerous
   false positives.

3) In textutils, od should open its input in binary mode.

4) ftruncate() changes the file position via SetFilePointer but doesn't
   restore it.

5) truncate() doesn't seem to exist.  At least, I can't find it.

6) After an lseek past EOF, writes needs to fill in the hole created.

   A somewhat simple-minded patch is attached.  It sets a flag on
   lseeks which look like they might go past EOF, and write checks the
   flag to find out for sure.  Successful reads and writes clear the
   flag.  I can think of several potential problems with this approach:
     - Not sensitive to external changes in file length.  A write() /
       truncate() / write() sequence (where the truncate erases some
       or all of the data from the first write) may leave garbage in
       the hole in the file.  I don't know why anyone would do this.
     - Race conditions with multiple processes.  In particular, if
       process 0 is writing odd numbered blocks of a file while
       process 1 is simultaneously writing even numbered blocks, both
       using lseek to skip the intervening blocks, this scheme is
       likely to clobber data even though this probably works just
       fine on Unix.
     - Doesn't account for the fact that it seems to be needed only
       for Win95.  It seems that NT may not have this problem?

-Vince Del Vecchio
[email-masked]

-----------------------------------------------------------------------------

diff -rc cdk.orig/winsup/fhandler.cc cdk/winsup/fhandler.cc
*** cdk.orig/winsup/fhandler.cc	Tue Apr 29 16:33:30 1997
--- cdk/winsup/fhandler.cc	Sun Jul 27 15:43:22 1997
***************
*** 208,213 ****
--- 208,215 ----
  	}
      }
  
+   if (bytes_read)
+     fpos_ = 0;
    return bytes_read;
  }
  
***************
*** 217,225 ****
    int len = (sizeof (access_) + sizeof (handle_) + sizeof (w_binary_) +
              sizeof (r_binary_) + sizeof (close_exec_p_) +
              sizeof (readahead_valid_) + sizeof (readahead_) +
!             sizeof (append_p_) + sizeof (rpos_) + sizeof (rsize_) +
!             sizeof (had_eof_) + sizeof (symlink_p_) + sizeof (execable_p_) +
!             sizeof (namehash_));
    
    if (buf == 0)
      return len;
--- 219,227 ----
    int len = (sizeof (access_) + sizeof (handle_) + sizeof (w_binary_) +
              sizeof (r_binary_) + sizeof (close_exec_p_) +
              sizeof (readahead_valid_) + sizeof (readahead_) +
!             sizeof (append_p_) + sizeof(fpos_) + sizeof (rpos_) +
!             sizeof (rsize_) + sizeof (had_eof_) + sizeof (symlink_p_) +
!             sizeof (execable_p_) + sizeof (namehash_));
    
    if (buf == 0)
      return len;
***************
*** 240,245 ****
--- 242,249 ----
    buf += sizeof (readahead_);
    memcpy (buf, (char *) &append_p_, sizeof (append_p_));
    buf += sizeof (append_p_);
+   memcpy (buf, (char *) &fpos_, sizeof (fpos_));
+   buf += sizeof (fpos_);
    memcpy (buf, (char *) &rpos_, sizeof (rpos_));
    buf += sizeof (rpos_);
    memcpy (buf, (char *) &rsize_, sizeof (rsize_));
***************
*** 277,282 ****
--- 281,288 ----
    buf += sizeof (readahead_);
    memcpy ((char *) &append_p_, buf, sizeof (append_p_));
    buf += sizeof (append_p_);
+   memcpy ((char *) &fpos_, buf, sizeof (fpos_));
+   buf += sizeof (fpos_);
    memcpy ((char *) &rpos_, buf, sizeof (rpos_));
    buf += sizeof (rpos_);
    memcpy ((char *) &rsize_, buf, sizeof (rsize_));
***************
*** 307,312 ****
--- 313,321 ----
          raise (SIGPIPE);
        return -1;
      }
+ 
+   if (bytes_written)
+     fpos_ = 0;
    return bytes_written;
  }
  
***************
*** 423,428 ****
--- 432,438 ----
    set_close_on_exec(0);
    symlink_p_ = 0;
    execable_p_ = 0;
+   fpos_ = 0;
    rpos_ = 0;
    had_eof_ = 0;
    rsize_ = -1;
***************
*** 606,611 ****
--- 616,668 ----
  
    if (append_p_)
      SetFilePointer (get_handle(), 0, 0, FILE_END);
+   else if (fpos_)
+     {
+       DWORD eof = GetFileSize (get_handle(), 0);
+       if (eof == (DWORD) -1)
+ 	{
+ 	  paranoid_printf ("GetFileSize (%d) failed in write\n", get_handle());
+ 	  __seterrno();
+ 	  return -1;
+ 	}
+       if (fpos_ > eof)
+ 	{
+ 	  /* Current file position is past EOF.  Fill the hole. */
+ 	  unsigned long target_eof = fpos_;
+ 	  debug_printf ("in write, filling hole from %u to %u\n",
+ 			eof, target_eof);
+ 	  if (SetFilePointer (get_handle(), eof, 0, FILE_BEGIN) == (DWORD) -1)
+ 	    {
+ 	      paranoid_printf ("SetFilePointer (%d, %u) failed in write\n",
+ 			       get_handle(), eof);
+ 	      __seterrno();
+ 	      return -1;
+ 	    }
+ 
+ 	  char emptybuf[CHUNK_SIZE];
+ 	  bzero (emptybuf, CHUNK_SIZE);
+ 	  while (target_eof > eof)
+ 	    {
+ 	      /* Note that raw_write clears fpos_ on success. */
+ 	      res = raw_write (emptybuf,
+ 			       target_eof - eof > CHUNK_SIZE ? CHUNK_SIZE
+ 			       : target_eof - eof);
+ 	      eof += res;
+ 	      if (res != -1)
+ 		continue;
+ 
+ 	      /* Write error.  Try to restore the file pointer. */
+ 	      fpos_ = SetFilePointer (get_handle(), target_eof, 0, FILE_BEGIN);
+ 	      if (fpos_ == (DWORD) -1)
+ 		{
+ 		  paranoid_printf ("SetFilePointer (%d, %u) failed on restore "
+ 				   "in write\n", get_handle(), target_eof);
+ 		  fpos_ = 0; /* Moved the file pointer (bad), but now <= EOF */
+ 		}
+ 	      return -1;
+ 	    }
+ 	}
+     }
  
    if (get_w_binary ())
      {
***************
*** 746,751 ****
--- 803,833 ----
      {
        __seterrno ();
      }
+   else
+     {
+       /* We need to make a note of this seek, so that if we next try to do
+ 	 a write and we are past EOF, we can fill in the gap.  If we can
+ 	 determine that we are before EOF, record that, otherwise, set
+ 	 the flag that says we may have past EOF. */
+       if (offset == 0)
+ 	{
+ 	  /* Offset 0 from start or end of file is within file.
+ 	     Offset 0 from current position means no change. */
+ 	  if (win32_whence != FILE_CURRENT)
+ 	    fpos_ = 0;
+ 	}
+       else if (offset < 0
+ 	       && (win32_whence == FILE_END
+ 		   || (win32_whence == FILE_CURRENT && fpos_ == 0)))
+ 	{
+ 	  /* Negative offset from EOF is within file, as is negative offset
+ 	     from current position if current position is okay. */
+ 	  fpos_ = 0;
+ 	}
+       else
+ 	fpos_ = res;
+     }
+ 
    return res;
  }
  
***************
*** 947,952 ****
--- 1029,1035 ----
    readahead_valid_ = 0; 
    readahead_ = 0; 
    append_p_ = 0;
+   fpos_ = 0;
    rpos_ = 0;
    rsize_ = 0;
    had_eof_ = 0;
diff -rc cdk.orig/winsup/fhandler.h cdk/winsup/fhandler.h
*** cdk.orig/winsup/fhandler.h	Thu Apr 24 20:59:18 1997
--- cdk/winsup/fhandler.h	Fri Jul 25 18:06:03 1997
***************
*** 56,61 ****
--- 56,65 ----
  
    char append_p_; /* Set if always appending */
  
+   /* Non-zero if we may have seeked past EOF.
+      Used to manually fill "holes" created by subsequent writes. */
+   unsigned long fpos_;
+ 
    int rpos_; /* Used in text reading */
    int rsize_; 
  
-
For help on using this list (especially unsubscribing), send a message to
"[email-masked]" with one line of text: "help".