Re: lseek(2) problems

[VOROSKOI Andras <voroskoi@xxxxxxxxx>]

On Mon, Jun 02, 2008 at 10:13:40AM -0700, Matthew Dillon wrote:
>     This looks really easy to fix.  I'll do it right now.

Thank you!

But it revealed an other problem. I should have send the whole testcase
i think. So I attach the whole testcase, the problamatic part is line 55
and 69.

I've also attached a possible fix for this second issue.

-- 
voroskoi

/* Test of lseek() function.
   Copyright (C) 2007-2008 Free Software Foundation, Inc.

   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; either version 3 of the License, or
   (at your option) any later version.

   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, see <http://www.gnu.org/licenses/>.  */

/* Written by Eric Blake, 2007.  */

#include <config.h>

#include <errno.h>
#include <stdio.h>
#include <unistd.h>

#define ASSERT(expr) \
  do									     \
    {									     \
      if (!(expr))							     \
        {								     \
          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
          fflush (stderr);						     \
          abort ();							     \
        }								     \
    }									     \
  while (0)

/* ARGC must be 2; *ARGV[1] is '0' if stdin and stdout are files, '1'
   if they are pipes, and '2' if they are closed.  Check for proper
   semantics of lseek.  */
int
main (int argc, char **argv)
{
  if (argc != 2)
    return 2;
  switch (*argv[1])
    {
    case '0': /* regular files */
      ASSERT (lseek (0, (off_t)2, SEEK_SET) == 2);
      ASSERT (lseek (0, (off_t)-4, SEEK_CUR) == -1);
      ASSERT (errno == EINVAL);
      errno = 0;
#if ! defined __BEOS__
      /* POSIX says that the last lseek call, when failing, does not change
	 the current offset.  But BeOS sets it to 0.  */
      ASSERT (lseek (0, (off_t)0, SEEK_CUR) == 2);
#endif
#if 0 /* leads to SIGSYS on IRIX 6.5 */
      ASSERT (lseek (0, (off_t)0, (SEEK_SET | SEEK_CUR | SEEK_END) + 1) == -1);
      ASSERT (errno == EINVAL);
#endif
      ASSERT (lseek (1, (off_t)2, SEEK_SET) == 2);
      errno = 0;
      ASSERT (lseek (1, (off_t)-4, SEEK_CUR) == -1);
      ASSERT (errno == EINVAL);
      errno = 0;
#if ! defined __BEOS__
      /* POSIX says that the last lseek call, when failing, does not change
	 the current offset.  But BeOS sets it to 0.  */
      ASSERT (lseek (1, (off_t)0, SEEK_CUR) == 2);
#endif
#if 0 /* leads to SIGSYS on IRIX 6.5 */
      ASSERT (lseek (1, (off_t)0, (SEEK_SET | SEEK_CUR | SEEK_END) + 1) == -1);
      ASSERT (errno == EINVAL);
#endif
      break;

    case '1': /* pipes */
      errno = 0;
      ASSERT (lseek (0, (off_t)0, SEEK_CUR) == -1);
      ASSERT (errno == ESPIPE);
      errno = 0;
      ASSERT (lseek (1, (off_t)0, SEEK_CUR) == -1);
      ASSERT (errno == ESPIPE);
      break;

    case '2': /* closed */
      /* Explicitly close file descriptors 0 and 1.  The <&- and >&- in the
         invoking shell are not enough on HP-UX.  */
      close (0);
      close (1);
      errno = 0;
      ASSERT (lseek (0, (off_t)0, SEEK_CUR) == -1);
      ASSERT (errno == EBADF);
      errno = 0;
      ASSERT (lseek (1, (off_t)0, SEEK_CUR) == -1);
      ASSERT (errno == EBADF);
      break;

    default:
      return 1;
    }
  return 0;
}

diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index b8c957a..0a9bba2 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -2264,7 +2264,10 @@ kern_lseek(int fd, off_t offset, int whence, off_t *res)
 		     vp->v_type == VCHR || vp->v_type == VBLK)) {
 			error = EINVAL;
 		}
-		fp->f_offset = new_offset;
+		/* According to POSIX failed lseek should not change the
+		 * offset */
+		if (error != EINVAL)
+			fp->f_offset = new_offset;
 	}
 	*res = fp->f_offset;
 done: