Mail Archives: djgpp-workers/2000/11/27/06:45:51
On Mon, 27 Nov 2000, Peter J. Farley III wrote:
> Attached are three diff files which I *think* should come through as
> text and not mime-encoded, but *please* let me know how they reach you.
They seem to be received okay.
> src.dif contains all the patches for the /src tree, include.dif
> contains all the patches for the include tree (including the file I
> missed the last time, /include/libc/getdinfo.h) and tests.dif contains
> all the patches for the /tests tree.
Thanks!
Please don't be discouraged by the comments below: they are mostly
minor technicalities.
> If anyone here thinks we should just implement it anyway, I'm prepared
> to hear arguments in favor, but I won't be able to do a table-based
> implementation myself, as my copious spare time :) is severely
> depleted.
Perhaps it would be a good idea to implement lockf as a simple wrapper
around fcntl. We could add the tables later, if they are needed.
Unless programs which use lockf *assume* that the tables are available
and *rely* on them, that is.
(Btw, the patch you sent does include wc204 changes that tell lockf
_is_ available.)
> +v2.03) now work, with two exceptions: (1) command F_GETLK will not
Preprocessor symbols such as F_GETLK should have a @code markup...
> +return values in the 'struct flock' parameter identifying what lock
...and so should "struct flock". In general, anything that is put
into a source code verbatim should be in @code{}.
> +@itemize @plus{}
> +@item
> +lock_req->l_whence = SEEK_SET
> +@item
> +lock_req->l_start == 0
> +@item
> +lock_req->l_len == 0
> +@end itemize
I think "@table @code" would be better here. Also, why didn't you use
"==" in the first condition as well?
> +This is because DOS/Windows only supports read locks on a per-file
> +basis, so any attempted use of a read lock is an error, UNLESS all of
In Texinfo, there's no need to use CAPS for emphasis; use
@strong{unless} instead.
> +@cindex fcntl commands F_GETFD and F_SETFD
Please add @code markup to F_SETFD and friends in @cindex entries also
(@findex and @vindex entries already have an implicit @code markup).
> +@findex fcntl commands F_GETFL and F_SETFL
The @findex entries which combine C symbols and plain text should
arrange for the plain text to be in the normal typeface, like this:
@findex fcntl AT r{ commands }F_GETFL AT r{ and }F_SETFL
@r{} causes an escape into the normal Roman typeface.
> +these functions are still effective no-ops, since DOS/Windows does not
> +allow flags to be modified on an open file, except for F_SETFL to remove
> +the O_NONBLOCK flag, which is not supported by DJGPP anyway.
> +
> +@item
> +Documentation has been updated to reflect the new functionality and a
> +test program has been provided.
> +@end itemize
This piece of docs is too detailed for wc204.txi, which should only
have short descriptions of each change. Please move these details
into fcntl.txh (which doesn't mention some of the information you put
in wcfcntl.txi).
> -int dosexterr(struct DOSERROR *p_error);
> +int dosexterr(struct _DOSERROR *p_error);
Why this change? `dosexterr' is a compatibility function: it mimics
its Borland namesake, which uses DOSERROR, not _DOSERROR. Using
_DOSERROR would be incompatible.
> -void main(void)
> +int main(void)
> @{
> FILE *fp;
> struct _DOSERROR de;
This should be changed to DOSERROR, for the same reason (yes, I know:
it's not your bug ;-).
> +This function accepts the extended error number from DOS (e.g., from
> +the return value from function @ref{dosexterr} or from the value of
> +@ref{_doserrno} after a failed DOS call)
Please don't use @ref{} like this. It might appear as a nifty trick
(you combine a cross-reference with the text), but it looks awkward in
Info format:
"...from the value of *Note {_doserrno} after a failed DOS call"
and simply unreadable in the printed output:
"...from the value of Section 2.10 [_doserrno], page 123, after a
failed DOS call"
Also, you need a comma or a period after @ref{}, or else some Info
browsers will become confused.
Here's how I suggest to write such fragments:
This function accepts the extended error number from DOS (e.g., from
the return value from function @ref{dosexterr} or from the value of
@code{_doserrno}, see @ref{_doserrno}, after a failed DOS call)
> +For a list of the strings returned for each error number, see the
> +list at @ref{dosexterr}.
Same here (and in a few more places).
> +@example
> +#include <dos.h>
> +
> +char *
> +dostrcls(doserrclass);
> +@end example
Please don't put a newline between the function return type and its
name in .txh files. Instead, write this:
@example
char *dostrcls (doserrclass);
@end example
> +This function accepts the extended error class number from DOS (i.e.,
> +from the returned error class value from function @ref{dosexterr} after
> +a failed DOS call) and returns the string which describes that error
> +class number. This function is a DOS analogue of the ANSI function
> +@ref{strerror}, and can be used to print a descriptive message
> +corresponding to the error class.
I'm not sure you can say that dostrcls is an analogue of strerror.
dostrerr is such an analogue, but dostrcls has no direct analogue;
it's an extension. The same pertains to dostract and dostrloc.
> +const char *__dos_errlist [] = {
> +/* 00h (0) */ "No error",
> +/* 01h (1) */ "Function number invalid",
[snip]
> +/* 67h (103)*/ "(MSCDEX) Not High Sierra or ISO-9660 format",
> +/* 68h (104)*/ "(MSCDEX) Door open"
> +/* (End of list) */ };
This list is incomplete; there are many more error codes, all the way
to FFh. They are all present in version 61 of RBIL, which I suggest
to use as the source.
> +const char *__dos_errclass [] = {
> +/* 01h (01) */ "Out of resource (storage space or I/O channels)",
> +/* 02h (02) */ "Temporary situation (file or record lock)",
> +/* 03h (03) */ "Authorization (denied access)",
> +/* 04h (04) */ "Internal (system software bug)",
> +/* 05h (05) */ "Hardware failure",
> +/* 06h (06) */ "System failure (configuration file missing or incorrect)",
> +/* 07h (07) */ "Application program error",
> +/* 08h (08) */ "Not found",
> +/* 09h (09) */ "Bad format",
> +/* 0Ah (10) */ "Locked",
> +/* 0Bh (11) */ "Media error",
> +/* 0Ch (12) */ "Already exists",
> +/* 0Dh (13) */ "Unknown"
> +/* (End of list) */ };
Same here: there are two more entries in RBIL v61.
> +static char *
> +_err_unknown(int errnum)
> +{
> + static char ebuf[40]; /* 64-bit number + slop */
> + char *cp;
> + int v=1000000, lz=0;
> +
> + strcpy(ebuf, "Unknown error: ");
> + cp = ebuf + 15;
Please avoid magic constants such as 15. Instead, make the string
"Unknown error: " be a const char array and use sizeof. This prevents
possible bugs due to miscounts, and also automagically DTRT if the
constant string is ever modified.
> +dostrerr(int errnum)
> +{
> + if (errnum >= 0 && errnum < __dos_nerr)
> + return(unconst(__dos_errlist[errnum], char *));
> + else
> + return(_err_unknown(errnum));
> +}
> +
> +char *
> +dostrcls(int errnum)
> +{
> + if (errnum >= 0 && errnum < __dos_ncls)
> + return(unconst(__dos_errclass[errnum], char *));
> + else
> + return(_err_unknown(errnum));
> +}
I wonder whether it will be a better design to return all this info as
a single struct, instead of having 4 separate functions. It strikes
me that whoever wants the human-redable description of what _dosexterr
returns, will want all of the info, not parts of it. It also seems to
be more consistent with _dosexterr itself, which returns all of the
codes at once.
Thoughts?
> +/* Changed 11/25/2000, 21h (33) EPERM -> EACCES */
> +/* 21h (33) lock violation needs EACCES errno in fcntl */
> +/* Peter J. Farley III <pjfarley AT banet DOT net */
There's no need for these comments: that's what CVS is for. If you
think it's important to mention why 21h is mapped to EACCES, please
put a comment near the code which does that, saying that fcntl needs
it.
> + /* Get the JFT entry address for this handle. */
> + regs.x.ax = 0x1220;
> regs.x.bx = fd;
> - __dpmi_int(0x21, ®s);
> + __dpmi_int(0x2f, ®s);
> +
> +
> if (regs.x.flags & 1)
> + {
> + errno = ENOSYS;
I don't think it's right to set errno to ENOSYS here. This call is
supported by DOS 3.0 and later, so ENOSYS is inappropriate (since
DJGPP doesn't work on DOS versions before 3). RBIL says that 2F1220
can return 6 in AL (invalid file handle), which should be mapped to
EBADF. Simply use __doserr_to_errno, it will DTRT. There are several
more places where I think you need to make such a change.
> + /* Get the SFT entry number for this handle. */
> + es = regs.x.es;
> + di = regs.x.di;
> + index = _farpeekb(_dos_ds, es * 16 + di);
>
> - return 1;
> +
> + /* Now get the address of the entry. */
> + regs.x.ax = 0x1216;
> + regs.x.bx = index;
> + __dpmi_int (0x2f, ®s);
Careful: if index is 255 (FFh), it means the file handle is not open
(see RBIL), and you should return EBADF in errno instead of feeding
2F1216 with an invalid index. (It is true that 2F1216 is documented
to fail if the index is grater than what FILES= says, but we are
dealing with undocumented internal OS calls, so playing safe is always
a good idea.)
> + new_dev_info = _get_dev_info(fd);
> +
> + /* If the dev info words are equal, then the documented
> + interface can't be used to set the inheritance bit. */
> + return (new_dev_info == dev_info) ? 1 : 0;
The above comment seems to be misleading: acrually, if the info words
are equal, the documented interface *can* be used to set inheritance
bit. Or am I missing something?
> + case F_SETLKW:
> + {
> + struct flock *lock_req = NULL; /* shut up -Wall */
> + int ret = -1;
> + off_t pos, cur_pos, lock_pos;
> + off_t len;
> +
> +
> + cur_pos = lseek(fd, 0, SEEK_CUR);
This should check if cur_pos is -1, and if so, bail out. This might
happen, e.g., if fd is invalid.
> + va_start (ap, cmd);
> + lock_req = va_arg(ap, struct flock *);
> + va_end (ap);
> + lock_pos = lseek (fd, lock_req->l_start, lock_req->l_whence);
lock_pos can also be -1 (if the file is a device, for example).
> + lseek (fd, cur_pos, SEEK_SET);
This doesn't check for errors, but should IMHO.
> + /* DOS/Windows only support read locks on a per-file basis,
> + so any attempted use of a read lock is an error,
> + UNLESS all of these conditions are true, which makes the
> + request into a per-file lock equivalent to F_WRLCK:
> +
> + lock_req->l_whence = SEEK_SET
> + lock_req->l_start == 0
> + lock_req->l_len == 0
> + */
Hmm... I cannot figure out this comment, in particular what do you
mean by ``only supports use of read locks on a per-file basis''. It
is also not clear why do you insist on l_whence being SEEK_SET:
clearly, the same conditions, whatever they are, can be created by a
suitably chosen values of an offset with SEEK_CUR and SEEK_END, no?
Could you please explain this some more?
> @item F_SETFL
> Set the open mode and status flags associated with the handle @var{fd}.
> -This always fails and sets @code{errno} to @code{ENOSYS}, since DOS and
> -Windows don't allow to change the descriptor flags after the file is
> -open.
> +This fails in all but one case, and sets @code{errno} to @code{ENOSYS},
> +since DOS and Windows don't allow changing the descriptor flags after
> +the file is open. The one allowed case is for @code{O_NONBLOCK}, since
> +DJGPP doesn't support it anyway.
This doesn't explain what is the result when O_NONBLOCK is used. Does
the function always succeed or always fail? Let's not ask the user to
guess (they will guess wrong ;-).
> @item F_SETLKW
> Same as @code{F_SETLK}, but if the lock is blocked, the call will wait
> -until it is unblocked and the lock can be applied. This is unsupported
> -and will always fail.
> +(using @code{__dpmi_yield()}) until it is unblocked and the lock can be
Please add a cross-reference to __dpmi_yield's docs.
> +@item LOCK_SH
> +Shared lock. More than one process may hold a shared lock for a given
> +file at a given time.
> +
> +@item LOCK_EX
> +Exclusive lock. Only one process may hold an exclusive lock for a given
> +file at a given time.
But DOS doesn't really distinguish between shared and exclusive locks,
does it? If it doesn't, please say so in the docs.
Thanks again for your efforts.
- Raw text -