Mail Archives: djgpp-workers/2000/11/28/02:16:29
On Mon, 27 Nov 2000, Peter J. Farley III wrote:
> >Please don't be discouraged by the comments below: they are mostly
> >minor technicalities.
>
> Whew! But there sure are a lot of them...
You wrote a lot of code ;-)
> >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.
>
> Yes, but in the sense that they return a string in response to an error
> code, they are an analogue, IMHO. However, I can see your point. I
> have no real problem changing the class/action/locus descriptions to
> say something like "... returns a string describing the ..., similar to
> the way strerror returns a string for values of @code{errno}...". Does
> that sound OK?
Yes.
> >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.
>
> Well, I used the comments in D_EXTERR.C as the source. I will try to
> find compatible text in RBIL 61.
The text you saw was lifted almost verbatim from an older version of
RBIL. So formatting what's in RBIL v61 should not be a problem.
> >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.
>
> Well, this is an exact copy of the code for strerror(), so I guess that
> needs fixing too.
Yes.
> OTOH, if someone has bothered to look up how to call dosexterr(), maybe
> it's not so much to ask to use another struct to get the strings
> describing the errors, especially if the member names are similar to
> the ones returned by dosexterr(). For example,
>
> struct DOSERROR_STR {
> char *exterror_str;
> char *class_str;
> char *action_str;
> char *locus_str;
> };
That's what I had in mind.
> > > +/* 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.
>
> Since I don't have access to CVS, nor have I ever used it, so I didn't
> know that. My practice has always been to comment what I change where
> I change it. I put it at the front because the change is in
> the initialization of the array which immediately follows. However,
> if CVS handles this function, I can remove the comment.
The CVS records a ``checkin comment'', a piece of text, with each
update. It is a usual practice, at least when I commit code
contributed by others, to say there who contributed it. The date of
change, as well as the diffs for the change, are recorded by CVS
automatically.
My point was that the information about who changed the code and when
is not needed in the source. A person who reads the source might be
interested to know why a particular mapping was taken, but it is
mostly irrelevant who did that and when. Thus, a comment near the
particular mapping which says why it was done is all that is required.
But if you feel that this comment, as you wrote it, should be
retained, I don't have any strong objections.
> >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.
>
> This is Mark E.'s original code, I did not change it. I can make the
> changes you suggest.
Unless Mark objects, please do.
> >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.)
>
> Again, this is Mark E.'s code, I have *not* done the research to have
> this knowledge. However, if what you are saying is that the value
> needs to be tested for x'FF' and to return EBADF if equal, that can be
> done.
Yes, that's what I'm saying. RBIL clearly says that the value of FFh
is an indication of an invalid handle. This is also confirmed by what
I know from other sources: JFT, the table in the program's PSP where
DOS keeps, for each open handle, the index of the corresponding SFT
entry, has an FFh for each unopen handle. DOS only allows for a
maximum of 255 handles per process, from 0 to 254; 255 is left free
for this purpose.
> I already saw the subsequent comments about
> using llseek() and Offset_t, so I'll change the calls and variables
> appropriately. That will also mean introducing some intermediate
> variables, since I don't think we should change the types of the
> members of the struct flock.
It would be interesting to see whether Int 21h/AH=5Ch works with FAT32
in a way that supports regions and offsets as large as FAT32 can allow
in principle. RBIL is silent on this, as far as I could see. Can
someone please test that, on both DOS and Windows? (Does SHARE.EXE
which comes with DOS 7.10 and later even support FAT32?)
If large regions can be locked, I think we need to consider changing
the declaration of struct flock accordingly. What trouble can this
make, given that we only widen the types? Do other systems which
support large files [Linux, Solaris] use different structures in fcntl
locking in this case?
> > > + /* 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?
>
> The first part (the part you don't understand) is Mark's comment, not
> mine. I took it as truth and did not change it.
Mark, could you please enlighten us?
> How would one go about determining whether an arbitrary combination
> "covered the whole file"?
You use lseek to seek to the starting offset, then look at lseek's
return value and compare it with zero. Then you lseek to the end of
the region and compare lseek's return value with the result of
"lseek(fd,0,SEEK_END);".
> And thank you for your diligence in reviewing it all. Forgive me in
> advance if I don't catch everything in pass #2.
Don't worry, I'm used to it ;-)
- Raw text -