Date: Mon, 24 Nov 1997 21:32:35 -0500 (EST) Message-Id: <199711250232.VAA28273@delorie.com> From: DJ Delorie 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 Precedence: bulk > 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 > ! #include > #include > #include > > --- 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 > #include 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 > #include > > --- 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 > #if defined(__DJGPP__) Again, do not change my copyright lines. > *************** > *** 1,4 > /* Copyright (C) 1995 DJ Delorie, see COPYING.DJ for details */ > #include > #include > > > --- 6,10 ----- > > #include > + #if defined(__DJGPP__) > #include > #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 > #include > > --- 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 > #include 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 > > static int num_fds; > static __FSEXT_Function **func_list; > > --- 24,31 ----- > #include > > + #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 ! func_list[i] = 0; > } > func_list[_fd] = _function; > > --- 109,113 ----- > return 1; > for (i=old_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 > #include /* 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 > #include /* For stat() */ Another copyright change. > *************** > *** 8,14 > #include /* 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 /*// 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 > #include > - > #include > > > --- 7,10 ----- > #include > #include > #include > #include 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 > ! #include > #include > #include > > --- 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 > 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.