On Thu, Apr 18, 2024 at 02:33:54AM +0900, Masatake YAMATO wrote:
> > int main(int argc, char *argv[])
> > {
> > struct ul_timer timer;
> > @@ -140,6 +175,7 @@ int main(int argc, char *argv[])
> > int no_fork = 0;
> > int status;
> > int verbose = 0;
> > + int use_fcntl_ofd = 0;
What about to introduce "lock_method" variable and define
enum {
LOCK_BY_FLOCK
LOCK_BY_FCNTL_OFD
};
later you can easily extend it by LOCK_BY_FCNTL_POSIX. IMHO the code
will be more readable.
> > struct timeval time_start = { 0 }, time_done = { 0 };
> > /*
> > * The default exit code for lock conflict or timeout
> > @@ -149,7 +185,8 @@ int main(int argc, char *argv[])
> > char **cmd_argv = NULL, *sh_c_argv[4];
> > const char *filename = NULL;
> > enum {
> > - OPT_VERBOSE = CHAR_MAX + 1
> > + OPT_VERBOSE = CHAR_MAX + 1,
> > + OPT_FCNTL_OFD,
> > };
> > static const struct option long_options[] = {
> > {"shared", no_argument, NULL, 's'},
> > @@ -163,6 +200,7 @@ int main(int argc, char *argv[])
> > {"close", no_argument, NULL, 'o'},
> > {"no-fork", no_argument, NULL, 'F'},
> > {"verbose", no_argument, NULL, OPT_VERBOSE},
> > + {"fcntl-ofd", no_argument, NULL, OPT_FCNTL_OFD},
I agree that the sort name --fcntl sounds better and it's extendable.
> > {"help", no_argument, NULL, 'h'},
> > {"version", no_argument, NULL, 'V'},
> > {NULL, 0, NULL, 0}
> > @@ -217,6 +255,11 @@ int main(int argc, char *argv[])
> > if (conflict_exit_code < 0 || conflict_exit_code > 255)
> > errx(EX_USAGE, _("exit code out of range (expected 0 to 255)"));
> > break;
> > +#ifdef HAVE_FCNTL_OFD_LOCKS
> > + case OPT_FCNTL_OFD:
> > + use_fcntl_ofd = 1;
> > + break;
> > +#endif
> > case OPT_VERBOSE:
> > verbose = 1;
> > break;
> > @@ -234,6 +277,13 @@ int main(int argc, char *argv[])
> > errx(EX_USAGE,
> > _("the --no-fork and --close options are incompatible"));
> >
> > + /*
> > + * For fcntl(F_OFD_SETLK), an exclusive lock requires that the
> > + * file is open for write.
> > + */
> > + if (use_fcntl_ofd && type == LOCK_EX)
> > + open_flags = O_WRONLY;
> > +
> > if (argc > optind + 1) {
> > /* Run command */
> > if (!strcmp(argv[optind + 1], "-c") ||
> > @@ -280,9 +330,15 @@ int main(int argc, char *argv[])
> >
> > if (verbose)
> > gettime_monotonic(&time_start);
> > - while (flock(fd, type | block)) {
> > + while (use_fcntl_ofd ? do_fcntl_lock(fd, type, block) : flock(fd, type | block)) {
Maybe we can move the locking to a function
do_lock(fd, method, type, block)
and hide the necessary flock and fcntl details there, instead of
trying to do it in main().
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
