Date: Tue, 12 Dec 2000 11:30:23 +0200 (IST) From: Eli Zaretskii X-Sender: eliz AT is To: "Peter J. Farley III" cc: djgpp-workers AT delorie DOT com Subject: Re: Locking fcntl changes #2 In-Reply-To: <5.0.2.1.0.20001211221616.00a71e51@pop5.banet.net> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Reply-To: djgpp-workers AT delorie DOT com Errors-To: nobody AT delorie DOT com X-Mailing-List: djgpp-workers AT delorie DOT com X-Unsubscribes-To: listserv AT delorie DOT com Precedence: bulk On Mon, 11 Dec 2000, Peter J. Farley III wrote: > OK, Real Soon Now has arrived. I think I have incorporated all of the > suggestions and corrections that were made. Thanks! > In addition, after > these patches are accepted and applied, someone needs to test the FAT32 > functions with real 32-bit offsets on a file whose size is > 2GB. Actually, I would be glad if this were tested _before_ the changes are committed, if someone with access to a FAT32 machine can do that. If not, we will have to keep our fingers crossed... > All coded tests just use zero offsets. Isn't it better to try non-zero offsets as well? You could get them from `rand', for example. That would also allow an easy test on FAT32 systems, if you probe for FAT32 support and provide a special test for that, which actually tries to lock after 2GB. > +struct _DOSERROR_STR { > + char *exterror_str; > + #ifdef __cplusplus > + char *errclass_str; > + #else > + char *class_str; > + #endif > + char *action_str; > + char *locus_str; Why do we need the special __cplusplus case? Can't we always call that member errclass_str (and rename the other two to erraction_str and errlocus_str)? > + /* This call will lock the program in an endless loop, > + since it already has the lock. > + > + FIXME -- Need to add SIGINT interrupt handler and > + console output (syserr?) telling user to > + press CNTL-Break to continue the test. > + > + Commented out for now, replaced with message about bypass. > + > + errno = 0; > + retval = llockf(fd, F_LOCK, 0); > + printf("F_LOCK:retval=%d\n", retval); > + if ((retval < 0) && (errno != 0)) prdoserr(fd); > + > + */ I think the following should do what you want (but I didn't test this): #include #include void (*old_sigint)(int); jmp_buf sigint_jmp; void sig_catcher (sig) { longjmp (sigint_jmp, sig); } ... int sig; if ((sig = setjmp (sigint_jmp)) == 0) { old_sigint = signal (SIGINT, sig_catcher); if (old_sigint == SIG_ERR) printf ("F_LOCK: Bypassed because SIGINT cannot be caught.\n"); else { puts ("Entering an endless loop, press Ctrl-C to end it."); errno = 0; retval = llockf(fd, F_LOCK, 0); printf("F_LOCK:retval=%d\n(This should NOT have been printed!)\n", retval); if ((retval < 0) && (errno != 0)) prdoserr(fd); } } else printf ("F_LOCK interrupted by %s\n", sys_siglist[sig]); /* Restore the previous SIGINT handler. */ if (old_sigint != SIG_ERR) signal (SIGINT, old_sigint); ... > + case F_TEST: > + lock_req.l_type = F_WRLCK; > + /* Test the lock: return 0 if FD is unlocked; > + return -1, set errno to EACCES, if the lock cannot be obtained. */ > + if (fcntl (_fildes, F_GETLK64, &lock_req) < 0) > + return -1; > + if (lock_req.l_type == F_UNLCK) > + return 0; > + errno = EACCES; > + return -1; Some systems return EAGAIN if the region is already locked. I'm not sure what does the majority of systems do; could people please look on the Unix and GNU/Linux boxes they have access to and tell what they see in "man lockf"? > +The @code{llockf} command is a simplified interface to the locking > +facilities of @code{fcntl} (see @ref{fcntl} for more detailed > +information). "@ref{fcntl}" should be followed by a comma or a period (comma in this case). Didn't makeinfo print a warning? > +The @code{llockf} command performs exactly the same functions as > +@code{lockf} Please add an @xref to lockf here. > +The @code{llockf} command is intended to permit using @code{fcntl} > +functions with @var{size} values greater than 2^31 - 1. It is better to use @math{2^31 - 1}, in case someone prints the libc reference: it looks nicer. > +@var{size} is the number of contiguous bytes to be locked or unlocked. > +The resource to be locked starts at the current offset in the file and > +extends forward for a positive size and backward for a negative size ^^^^ ^^^^ These two occurences of "size" should have the @var markup, since you are talking about the function's argument. > +(the preceding bytes up to but not including the current offset). If > +size is zero ^^^^ And also here. > +@table @code > +@item F_TEST > +This function is used to detect if the specified section is already > +locked. @code{F_LOCK} and @code{F_TLOCK} both lock a section of a file > +if the section is available. @code{F_ULOCK} removes locks from a > +section of the file. The last two sentences don't belong to the description of F_TEST. > +@item F_LOCK > +@item F_TLOCK In a @table, all @item's but the first should be @itemx. > +The @code{llockf} utility will fail llockf is not a utility. > if one or more of the following are > +true: > + > +@table @code > +@item EBADF > +Parameter @var{fildes} is not a valid open file. > + > +@item EACCES > +Cmd is @code{F_TLOCK} or @code{F_TEST} and the section is already > +locked. > + > +@item EINVAL > +Parameter @var{function} is not one of the implemented functions. > +@end table This doesn't make clear that EBADF etc. are the values of `errno'. (The last tow comments pertain to both llockf and lockf's docs.) > +This function accepts the extended error structure from DOS (e.g., from > +the returned parameter from function @code{dosexterr}, see > +@ref{dosexterr}) and returns Need a comma after @ref{}. > +structure in the structure pointed to by the second parameter. This > +function is a DOS analogue of the ANSI function @ref{strerror}, This use of @ref{} is one of the bad ones I wrote in reply to your previous patches. > +/* B4h (180) */ "(MS-DOS 7.0) Lock count has been exceeded", > +/* B4h (180) */ "(NetWare4) Invalid segment number", One of these two strings should not appear: they are for the same code. The first one should be retained, I think (we don't care about NetWare). > +/* B5h (181) */ "(MS-DOS 7.0) A valid eject request failed", > +/* B5h (181) */ "(DOS 5.0-6.0,NetWare4) Invalid call gate", Same here. > +/* FEh (254) */ "Reserved", Should all those "Reserved" be replaced by "Unknow error: XX"? > + /* If l_len is zero, then the lock is to be set from l_start > + until the end-of-file. */ > + if (len == 0) > + { > + len = filelength(fd) - cur_pos; > + /* This should probably be an error. */ > + if (len <= 0) > + len = 1; > + } Perhaps the case where filelength returns a negative value (can it?) should indeed return an error, but if filelength returns zero (meaning the file is empty), it should succeed, I think, because fcntl allows to lock regions beyond the current EOF. The question is: does DOS/Windows allow that? Otherwise, it is not clear to me why the above snippet bumps len to 1. > + len = filelength(fd) - cur_pos; > + /* This should probably be an error. */ > + if (len <= 0L) > + len = 1L; Does filelength in the current CVS support FAT32? > +@example > +#include > + > +ret = fcntl (fd, F_SETFL, O_BINARY); /* This will fail, returning -1 */ > + /* and setting errno to ENOSYS */ > + > +ret = fcntl (fd, F_SETFL, O_NONBLOCK); /* This will succeed */ > + /* returning 0 */ > +@end example This should use @smallexample (its lines are too long for @example). > +A file is locked, not the file descriptor. So, dup (2) does not create > +multiple instances of a lock. "dup" should have a @code markup, and please also add an @xref. Also, we don't use the (n) notation for commands and functions (because the references aren't man pages), so please remove "(2)" in the above. > +Dos/Win9x does not support shared locks, but the underlying > +implementation (which uses the @code{F_SETLK} (non-blocking) or > +@code{F_SETLKW} (blocking) commands to @code{fcntl}, see @ref{fcntl}) This should use "@pxref", not "see @ref", because the latter needs a period or a comma after it. Last, but not least: thanks for all the work!