rad_unlockfd

[Brian Candler]

I noticed something odd in rad_unlockfd - both 2.x.x and master.

int rad_unlockfd(int fd, int lock_len)
{
...
        fl.l_type = F_WRLCK;
        fl.l_whence = SEEK_CUR;

        return fcntl(fd, F_UNLCK, (void *)&fl);

However, according to the Linux manpage for fcntl:

       F_SETLK (struct flock *)
              Acquire  a lock (when l_type is F_RDLCK or F_WRLCK) or release a
              lock (when l_type is F_UNLCK) on  the  bytes  specified  by  the
              l_whence,  l_start,  and l_len fields of lock.  If a conflicting
              lock is held by another process, this call returns -1  and  sets
              errno to EACCES or EAGAIN.

That is: to unlock, the command should be F_SETLK(W) and the l_type should
be F_UNLCK.  The OSX (BSD) manpage agrees.

On Linux, the constant F_UNLCK is 2, so calling fcntl(fd, F_UNLCK...)
is the same as calling fcntl(fd, F_SETFD...)

The other oddity is using SEEK_CUR for both lock and unlock. If you have
written to the file in the mean time, then the current file offset will have
changed, so you may end up unlocking a different byte range to the one you
locked.  I'd say SEEK_SET with offset 0 and length 0 (which locks or unlocks
the entire file, including past its end) is safest - at least when
unlocking.

As far as I can see, only rad_utmp calls this function, so the impact is not
huge.

Regards,

Brian.