Date: Mon, 7 Jun 1999 09:19:39 +0300 (IDT)
From: Eli Zaretskii <eliz AT is DOT elta DOT co DOT il>
X-Sender: eliz AT is
To: "Mark E." <snowball3 AT bigfoot DOT com>
cc: djgpp-workers AT delorie DOT com
Subject: Re: enhancements to fcntl.c
In-Reply-To: <199906031836.SAA75560@out1.ibm.net>
Message-ID: <Pine.SUN.3.91.990607091852.18114A@is>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Reply-To: djgpp-workers AT delorie DOT com
X-Mailing-List: djgpp-workers AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk


On Thu, 3 Jun 1999, Mark E. wrote:

> I've attached an updated version of fcntl.c.

Thanks!

I compiled and ran inherit.c with the new fcntl on DOS, Windows 95 and
on NT.  It worked on all of those systems.

Several comments on the code:

    static long
    get_dev_info(int fd)

Why couldn't you use _get_dev_info()?  It seems to be identical to
this local function.

    if (dev_info < 0)
    {
      errno = EBADF;
      return -1;
    }

The above (from the beginning of fcntl) is okay, but you don't need to
set errno, since get_dev_info() already does that.

    while (tofd < open_max)
    {
      /* If unable to get the device info for the handle,
         then the handle is not active and it can be used.  */
      if (!get_dev_info(tofd))
        break;

get_dev_info returns -1 when it fails, not zero.

Under F_DUPFD, I suggest to save and restore the previous value of
errno before you "return dup2(fd, tofd)".  That's because the loop
which looks for a free tofd will end up with EBADF, so the function
will overwrite the previous value of errno even if it succeeds, which
isn't nice.

Under F_SETFL, I suggest to fail with ENOSYS if they include the
O_NONBLOCK bit, since we don't support it.  Also, perhaps the
AUTO-COMMIT flag supported by 216C00h and 21716Ch could be translated
into O_SYNC flags (although it's non-Posix).

    case F_SETLKW:

Shouldn't this case wait for the lock to be removed, with some
busy-wait loop that issues __dpmi_yield()?  This assumes that there's
some way of detecting the fact that the _dos_{un,}lock call failed
because the file is already locked.  If we can't, perhaps it's better
to fail with ENOSYS?

    if (ret < 0)
      errno = ENOSYS;

(This is at the end of the locking section.)  errno should be set by
_dos_lock and _dos_unlock.  They currently don't do that, but that's a
bug which I will correct.  (While at that, I should also change the
entry in doserr2e.c that returns EPERM for DOS error 21h to return
EACCES instead, as this fits the Unix behavior better.)

There were a few other minor problems revealed by -Wall; see the diffs
below.  inherit.c didn't compile for me at first; diffs attached (and
I find the interchange of argc and argv confusing).

Are you planning to extend the test program for commands other than
F_DUPFD?  I would like to see whether the other commands, in
particular F_SETFL and F_GETFL, work on Windows 9X and NT.  And the
expanded test program could then go directly into djtstNNN.zip ;-)

> Note that changes to the access mode (O_RDONLY, O_WRONLY, O_RDWR)
> are ignored to obey the online Unix spec.

Yes, Posix says only O_APPEND and O_NONBLOCK can be changed.  SunOS
also allows the (non-standard) O_SYNC, so if we support it, we should
allow it as well.

*** src/libc/posix/fcntl/fcntl.c~	Sun Jun  6 09:20:44 1999
--- src/libc/posix/fcntl/fcntl.c	Sun Jun  6 18:06:42 1999
***************
*** 7,12 ****
--- 7,13 ----
  #include <stdarg.h>
  #include <unistd.h>
  #include <fcntl.h>
+ #include <io.h>
  #include <sys/fsext.h>
  #include <sys/movedata.h>
  #include <sys/farptr.h>
***************
*** 79,85 ****
       the handle's device info word.  */
    if (dev_info < 0)
    {
-     errno = EBADF;
      return -1;
    }
  
--- 80,85 ----
***************
*** 104,110 ****
        {
          /* If unable to get the device info for the handle,
             then the handle is not active and it can be used.  */
!         if (!get_dev_info(tofd))
            break;
          tofd++;
        }
--- 104,110 ----
        {
          /* If unable to get the device info for the handle,
             then the handle is not active and it can be used.  */
!         if (get_dev_info(tofd) == -1)
            break;
          tofd++;
        }
***************
*** 146,154 ****
      case F_SETFD:
      {
        unsigned int flag;
!       unsigned long entry_ptr;
!       unsigned short dev_info;
!       unsigned char mode;
        unsigned char use_set_dev_info = 1;
        __dpmi_regs regs;
  
--- 146,152 ----
      case F_SETFD:
      {
        unsigned int flag;
!       unsigned long entry_ptr = 0; /* shut up -Wall */
        unsigned char use_set_dev_info = 1;
        __dpmi_regs regs;
  
***************
*** 242,253 ****
      case F_SETLK:
      case F_SETLKW:
      {
!       struct flock *lock_req = va_arg(lock_req, struct flock *);
        int ret = -1;
!       off_t pos, file_len;
        off_t cur_pos = lseek (fd, 0, SEEK_CUR);
!       off_t lock_pos = lseek (fd, lock_req->l_start, lock_req->l_whence);
!       off_t len = lock_req->l_len;
  
        /* If l_len is zero, then the lock is to be set from l_start
           until the end-of-file. */
--- 240,257 ----
      case F_SETLK:
      case F_SETLKW:
      {
!       struct flock *lock_req = NULL; /* shut up -Wall */
        int ret = -1;
!       off_t pos;
        off_t cur_pos = lseek (fd, 0, SEEK_CUR);
!       off_t lock_pos;
!       off_t len;
! 
!       va_start (ap, cmd);
!       lock_req = va_arg(lock_req, struct flock *);
!       va_end (ap);
!       lock_pos = lseek (fd, lock_req->l_start, lock_req->l_whence);
!       len = lock_req->l_len;
  
        /* If l_len is zero, then the lock is to be set from l_start
           until the end-of-file. */
*** src/libc/posix/fcntl/inherit.c~	Sun Jun  6 09:20:52 1999
--- src/libc/posix/fcntl/inherit.c	Sun Jun  6 17:53:48 1999
***************
*** 1,4 ****
--- 1,5 ----
  #include <stdio.h>
+ #include <stdlib.h>
  #include <fcntl.h>
  #include <process.h>
  
***************
*** 36,44 ****
        fd = strtol(argc[index], NULL, 10);
  
        if (fcntl(fd, F_GETFD) == -1)
!         printf("Handle %s was NOT inherited.\n", fd_str);
        else
!         printf("Handle %s was inherited.\n");
        ++index;
      }
      printf("\n");
--- 37,45 ----
        fd = strtol(argc[index], NULL, 10);
  
        if (fcntl(fd, F_GETFD) == -1)
!         printf("Handle %s was NOT inherited.\n", argc[index]);
        else
!         printf("Handle %s was inherited.\n", argc[index]);
        ++index;
      }
      printf("\n");