delorie.com/archives/browse.cgi   search  
Mail Archives: djgpp-workers/1997/11/24/21:33:01

Date: Mon, 24 Nov 1997 21:32:35 -0500 (EST)
Message-Id: <199711250232.VAA28273@delorie.com>
From: DJ Delorie <dj AT delorie DOT com>
To: randym AT acm DOT org
CC: djgpp-workers AT delorie DOT com
In-reply-to: <3.0.1.32.19971124144947.007bed80@yacker.xiotech.com> (message
from Randy Maas on Mon, 24 Nov 1997 14:49:47 -0600)
Subject: Re: some proposed fsext changes

> 3. Minimize redundancy: the __FSEXT_get_handler and __FSEXT_set_handler
> calls are similar to __FSEXT_set_function and __FSEXT_get_function.  But

Better to extend the original calls than to add two new calls that are
almost identical.

> *** src/libc/posix/unistd/dup2.c~1	Sun Sep 29 09:20:56 1996
> --- src/libc/posix/unistd/dup2.c	Mon Nov 24 12:04:04 1997
> ***************
> *** 1,5
> ! /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
> ! #include <libc/stubs.h>
> ! #include <unistd.h>
>   #include <fcntl.h>
>   #include <dpmi.h>
> 
> --- 1,7 -----
> ! /* Copyright (C) 1997 DJ Delorie, see COPYING.DJ for details
> !    Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details
> !    Modified: 1997, Randall Maas: carved it to pieces.  Now reflects the call
> !    to _dup2()
> !   */
>   #include <fcntl.h>
>   #include <io.h>

This is unacceptable.  Please do NOT modify my copyright lines.  I
have a program that automatically updates them, and when you change
the syntax it confuses my program.

> ***************
> *** 9,13
>   
>   int
> ! dup2(int fd, int newfd)
>   {
>     __dpmi_regs r;
> 
> --- 9,13 -----
>   
>   int
> ! dup2(int existing_handle, int new_handle)
>   {
>     if (existing_handle == new_handle)

This change was not necessary.  Please don't make changes just for the
sake of prettiness - it makes it harder to find the real changes.

> ***************
> *** 11,28
>   dup2(int fd, int newfd)
>   {
> !   __dpmi_regs r;
> !   if (fd == newfd)
> !     return newfd;
> !   __file_handle_set(newfd, __file_handle_modes[fd] ^ (O_BINARY|O_TEXT));
> !   r.h.ah = 0x46;
> !   r.x.bx = fd;
> !   r.x.cx = newfd;
> !   __dpmi_int(0x21, &r);
> !   if (r.x.flags & 1)
> !   {
> !     errno = __doserr_to_errno(r.x.ax);
> !     return -1;
> !   }
> !   setmode(newfd, __file_handle_modes[fd]);
> !   return newfd;
>   }
> 
> --- 11,16 -----
>   dup2(int existing_handle, int new_handle)
>   {
> !   if (existing_handle == new_handle)
> !     return new_handle;
> !   return _dup2(existing_handle, new_handle);
>   }

I see no point in this.  dup2() worked just fine, and there is no
reason to add yet another function to libc.

> *** src/libc/fsext/fse_open.c~1	Sat Nov 25 18:49:58 1995
> --- src/libc/fsext/fse_open.c	Mon Nov 24 11:27:52 1997
> ***************
> *** 1,3
> ! /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
>   #include <stdlib.h>
>   #include <sys/fsext.h>
> 
> --- 1,8 -----
> ! /* Copyright (C) 1997 DJ Delorie, see COPYING.DJ for details
> !    Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details
> !    Modified, 1997 Randall Maas -- uses local fsext.h on non-DJGPP platforms,
> !         and sends a "NULL" state variable to the open handlers.
> !  */
> ! 
>   #include <stdlib.h>
>   #if defined(__DJGPP__)

Again, do not change my copyright lines.

> ***************
> *** 1,4
>   /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
>   #include <stdlib.h>
>   #include <sys/fsext.h>
>   
> 
> --- 6,10 -----
>   
>   #include <stdlib.h>
> + #if defined(__DJGPP__)
>   #include <sys/fsext.h>
>   #else

This is unacceptable.  DJGPP's sources are for DJGPP's use only.

> ***************
> *** 24,28
>   int
>   __FSEXT_call_open_handlers(__FSEXT_Fnumber _function_number,
> ! 			   int *rv, va_list _args)
>   {
>     FuncList *f;
> 
> --- 34,38 -----
>   int
>   __FSEXT_call_open_handlers(__FSEXT_Fnumber _function_number,
> !                          int *rv, va_list _args)
>   {
>     FuncList *f;

Apparently, you don't like my use of tabs either.

> *** src/libc/fsext/fsext.c~1	Sat Nov 25 17:48:12 1995
> --- src/libc/fsext/fsext.c	Mon Nov 24 11:41:10 1997
> ***************
> *** 1,3
> ! /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
>   #include <stdio.h>
>   #include <stdlib.h>
> 
> --- 1,11 -----
> ! /* Copyright (C) 1997 DJ Delorie, see COPYING.DJ for details
> !    Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details
> !    Modified 1997, Randall Maas -- some cpp checks for non-DJGPP environments
> ! 
> !    Note: __FSEXT_get_function and __FSEXT_set_function have the same
> !          parameters and semantics as version 2.01.  __FSEXT_get_handler will
> !          provide both the function pointer  and the state pointer.
> ! */
> ! 
>   #include <stdio.h>
>   #include <stdlib.h>

Another copyright change.

>   #if defined(WIN32)

This code should not be in DJGPP's library.  I refuse to support
Microsoft's compilers.

> ***************
> *** 9,12
>   #include <libc/dosio.h>
>   
>   static int num_fds;
>   static __FSEXT_Function **func_list;
> 
> --- 24,31 -----
>   #include <string.h>
>   
> + #if defined(WIN32)
> + #define bzero(a,b) memset(a, 0, b)
> + #endif
> + 
>   static int num_fds;
>   

This shouldn't be needed at all.  Just use memset() in the first place.

> ***************
> *** 44,47
>   
>     fd = r.x.ax;
>     __FSEXT_set_function(fd, _function);
>     return fd;
> 
> --- 70,78 -----
>   
>     fd = r.x.ax;
> + #else
> +   /* This is for non-DJGPP environments */
> +   fd= open("/dev/null", O_RDWR|O_CREAT);
> + #endif
> + 
>     __FSEXT_set_function(fd, _function);
>     return fd;

DJGPP does not support non-DJGPP environments.

> ***************
> *** 64,68
>         return 1;
>       for (i=old_fds; i<num_fds; i++)
> !       func_list[i] = 0;
>     }
>     func_list[_fd] = _function;
> 
> --- 109,113 -----
>         return 1;
>       for (i=old_fds; i<num_fds; i++)
> !       bzero(&func_list[i], sizeof(func_list[i]));
>     }
>     func_list[_fd].func = _function;

You should use memset(), not bzero().

> ***************
> *** 78,79
>     return func_list[_fd];
>   }
> 
> --- 132,145 -----
>     return 1;
>   }
> + 
> + void
> + __FSEXT_close_all (void)
> + {
> +   int I;
> + 
> +   if (!func_list) return;
> +   for (I = 0; I < num_fds; I++)
> +    if (func_list[I].func)
> +      _close(I);
> + }
> + 
> 

What is this for?  Doesn't libc's standard exit() close everything it
should?

> *** src/libc/posix/unistd/link.c~1	Wed May 10 02:13:46 1995
> --- src/libc/posix/unistd/link.c	Mon Nov 24 12:07:16 1997
> ***************
> *** 1,3
> ! /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
>   #include <libc/stubs.h>
>   #include <sys/stat.h>		/* For stat() */
> 
> --- 1,6 -----
> ! /* Copyright (C) 1997 DJ Delorie, see COPYING.DJ for details
> !    Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details
> !    Modified 1997, Randall Maas to be a wrapper to _link.
> ! */
>   #include <libc/stubs.h>
>   #include <sys/stat.h>		/* For stat() */

Another copyright change.

> ***************
> *** 8,14
>   #include <errno.h>		/* For errno */
>   
> - /* Of course, DOS can't really do a link.  We just do a copy instead,
> -    which is as close as DOS gets.  Alternatively, we could always fail
> -    and return -1.  I think this is slightly better. */
>   int
>   link(const char *path1, const char *path2)
> 
> --- 11,14 -----
>   #include <unistd.h>             /*// For read(), write(), etc. */
>   
>   int
>   link(const char *exists, const char *new)

Why did you remove the comments?

>
> ...
> 
> --- 14,18 -----
>   link(const char *exists, const char *new)
>   {
> !    return _link(exists, new);
>   }
>   

I fail to see the usefulness of this.

> ***************
> *** 5,9
>   #include <go32.h>
>   #include <dpmi.h>
> - 
>   #include <libc/dosio.h>
>   
> 
> --- 7,10 -----
>   #include <go32.h>
>   #include <dpmi.h>
>   #include <libc/dosio.h>
>   #include <unistd.h>

Please avoid cosmetic changes mixed in with real changes.

> ***************
> *** 9,13
>   
>   off_t
> ! lseek(int handle, off_t offset, int whence)
>   {
>     __dpmi_regs r;
> 
> --- 11,15 -----
>   
>   off_t
> ! lseek(int fd, off_t offset, int whence)
>   {
>     return _lseek(fd, offset, whence);

More cosmetic changes.  It also appears that your diff program isn't
doing correct diffs - note that there are two other lines that differ
in that chunk, and they're not marked, yet they appear below.  What
diff are you using?

> ***************
> *** 11,26
>   lseek(int handle, off_t offset, int whence)
>   {
> !   __dpmi_regs r;
> !   r.h.ah = 0x42;
> !   r.h.al = whence;
> !   r.x.bx = handle;
> !   r.x.cx = offset >> 16;
> !   r.x.dx = offset & 0xffff;
> !   __dpmi_int(0x21, &r);
> !   if (r.x.flags & 1)
> !   {
> !     errno = __doserr_to_errno(r.x.ax);
> !     return -1;
> !   }
> !   return (r.x.dx << 16) + r.x.ax;
>   }
> 
> --- 13,17 -----
>   lseek(int fd, off_t offset, int whence)
>   {
> !   return _lseek(fd, offset, whence);
>   }

I fail to see the usefullnes of this.

> ***************
> *** 25,26
>     return (r.x.dx << 16) + r.x.ax;
>   }
> 
> --- 15,17 -----
>     return _lseek(fd, offset, whence);
>   }
> + 
> 

Is this extra line necessary?

> *** src/libc/ansi/stdio/remove.c~1	Sat Aug 31 21:09:32 1996
> --- src/libc/ansi/stdio/remove.c	Mon Nov 24 12:10:28 1997
> ***************
> *** 1,5
> ! /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
> ! #include <libc/stubs.h>
> ! #include <io.h>
>   #include <stdio.h>
>   #include <fcntl.h>
> 
> --- 1,8 -----
> ! /*
> !    removes a named file
> !    Copyright (C) 1997 DJ Delorie, see COPYING.DJ for details
> !    Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details
> !    Modified: 1997, Randall Maas: Made into a wrapper for _remove
> !     */
>   #include <stdio.h>
>   

Never put commentary before the copyrights.

> ***************
> *** 10,14
>    
>   int
> ! remove(const char *fn)
>   {
>     __dpmi_regs r;
> 
> --- 8,12 -----
>   
>   int
> ! remove(const char *file_name)
>   {
>     return _unlink(file_name);

More cosmetic changes?

> ***************
> *** 12,50
>   remove(const char *fn)
>   {
> !   __dpmi_regs r;
> !   unsigned attr;
> !   int directory_p;
> !   int use_lfn = _USE_LFN;
> !  
> !   /* Get the file attribute byte.  */
> !   attr = _chmod(fn, 0);
> !   directory_p = attr & 0x10;
> !  
> !   /* Now, make the file writable.  We must reset Vol, Dir, Sys and Hidden bits 
> !      in addition to the Read-Only bit, or else 214301 will fail.  */
> !   _chmod(fn, 1, attr & 0xffe0);
> ! 
> !   /* Now delete it.  Note, _chmod leaves dir name in tranfer buffer. */
> !   if (directory_p)
> !     r.h.ah = 0x3a;		/* DOS Remove Directory function */
> !   else
> !     r.h.ah = 0x41;		/* DOS Remove File function */
> !   if(use_lfn) {
> !     r.h.al = r.h.ah;
> !     r.h.ah = 0x71;
> !     r.x.si = 0;			/* No Wildcards */
> !   }
> !   r.x.dx = __tb_offset;
> !   r.x.ds = __tb_segment;
> !   __dpmi_int(0x21, &r);
> !   if(r.x.flags & 1)
> !   {
> !     /* We failed.  Leave the things as we've found them.  */
> !     int e = __doserr_to_errno(r.x.ax);
> !  
> !     _chmod(fn, 1, attr & 0xffe7);
> !     errno = e;
> !     return -1;
> !   }
> !   return 0;
>   }
> 
> --- 10,13 -----
>   remove(const char *file_name)
>   {
> !   return _unlink(file_name);
>   }
> 

I fail to see the usefulness of this.

> *** src/libc/posix/unistd/unlink.S~1	Thu May 25 02:21:26 1995
> --- src/libc/posix/unistd/unlink.S	Mon Nov 24 13:13:24 1997
> ***************
> *** 1,3
> ! /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */
>   	.global	_unlink
>   _unlink:
> 
> --- 1,5 -----
> ! /* Copyright (C) 1997 DJ Delorie, see COPYING.DJ for details */
> ! /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details
> !    Modified 1997, Randall Maas to call _unlink() instead of remove */
>   	.global	_unlink
>   _unlink:
> ***************
> *** 2,4
>   	.global	_unlink
>   _unlink:
> ! 	jmp	_remove
> 
> --- 4,6 -----
>   	.global	_unlink
>   _unlink:
> !         jmp     __unlink
> 

This wasn't really needed at all.  If you want to optimize the stubs,
please do so in a separate message so that they can be reviewed and
applied separately.  Sending multiple changes in the same mail means
that both get rejected if either is unacceptable.

- Raw text -


  webmaster     delorie software   privacy  
  Copyright © 2019   by DJ Delorie     Updated Jul 2019