lockf: allow locking file descriptors

[kevans]

Summary

This is most useful inside a shell script, allowing one to lock just
portions of a script rather than having to wrap the entire script in a
lock.

PR: 262738
Co-authored-by: Daniel O'Connor <[e-mail]>
Sponsored by: Klara, Inc.

[0mp]

LGTM. The tests look good and the new logic of handling fdlock mode is also OK. I only have some feedback regarding documentation and error messages.

usr.bin/lockf/lockf.1

25

Please bump the date when committing.

220

It seems like we are waiting for 10 seconds in the example, not 5.

236

This is a bit confusing, because /tmp/my.lock is not actually a lock file, is it? The whole example is about locking fd 9, not /tmp/my.lock.

usr.bin/lockf/lockf.c

274

For consistency with other messages.

[darius-dons.net.au]

usr.bin/lockf/lockf.1

236

Yes, it is the lock file but the shell script has opened it, when lockf is forked the FD is passed to it so it then does the actual lock ioctl.

The upshot is that the script can do the locking without having to assume it is being called by lockf. It also means you can do more fine grained locking within the script.

Perhaps after "for it to become available." we could add "Note that the shell script opens the lock file and lockf is performing the lock call via the passed in file descriptor (9)".

[allanjude]

reviewed-by: allanjude

[kevans]

Address review commentary, add a little more explanation in the new example in
the manpage and make the error message more consistent.

[des]

usr.bin/lockf/lockf.1

236

Will this actually work? Shouldn't it be >> to ensure that if the lock file already exists, we lock the existing file rather than replacing it with another?

236

I meant to attach the above to line 243, I don't know why it got sent as a reply instead.

[kevans]

usr.bin/lockf/lockf.1

236

Agreed; I've added the recommended verbiage, and also added a small note about the interaction (or rather, non-interaction) with -w since the file's pre-opened by the shell.

236

My interpretation of the POSIX wording is that > must be open(... O_CREAT|O_TRUNC ...) ) rather than unlink() + open(... O_CREAT ...):

<<<
Output redirection using the '>' format shall fail if the noclobber option is set (see the description of set -C) and the file named by the expansion of word exists and is a regular file. Otherwise, redirection using the '>' or ">|" formats shall cause the file whose name results from the expansion of word to be created and opened for output on the designated file descriptor, or standard output if none is specified. If the file does not exist, it shall be created; otherwise, it shall be truncated to be an empty file after being opened.
<<<

I suspect that's the much more common implementation, but maybe it's worth calling out the assumption in the wording above as well