Change the values of the LOCK_* constants View as Markdown

On Sun, Apr 24, 2022 at 12:14 PM Ilija Tovilo tovilo.ilija@gmail.com
wrote:

Hi everyone

The issue was raised that PHPs LOCK_* constants don't match the Unix
LOCK_* constants.
https://github.com/php/php-src/pull/8429

// Unix
#define LOCK_SH 1
#define LOCK_EX 2
#define LOCK_NB 4
#define LOCK_UN 8

// PHP
#define PHP_LOCK_SH 1
#define PHP_LOCK_EX 2
#define PHP_LOCK_UN 3
#define PHP_LOCK_NB 4

Essentially, in PHPs binary representation UN doesn't get its own bit,
but is instead represented as 0b11. I'm guessing the reasoning was
that SH, EX and UN must not be combined, while they can all be
combined with NB. This avoids additional error handling when multiple
of those bits were to be set.

However, this has a downside of making checking of bits harder and
different from how you would do it in other languages.
https://3v4l.org/41ebV

We could update the PHP constants to match the Unix values of those
constants. Unfortunately, there seems to be a not insignificant number
of usages of flock with hard-coded integer values.

https://sourcegraph.com/search?q=context:global+file:%5C.php%24+count:100000+flock%5C%28%5C%24%5Ba-zA-Z0-9_%5D%2B%2C+%5B0-9%5D%2B%5C%29&patternType=regexp

(The regex engine of sourcegraph is flaky, but the majority of results
are correct)

The process of replacing these hard-coded values could be partially
automated with a few caveats.

1. The value must be direct ($flags = 1; flock($file, $flags); would not
   work)
2. The migration script would assume that flock is a global and not
   local function

Overall, I'm not completely sure this change is worth it since flock
flags are just passed and not read.

Let me know what you think.

Ilija

I think the current state of things here makes perfect sense. I might help
to think of it as a structure of the form:

struct {
unsigned lock_type : 2;
unsigned non_blocking : 1;
}

The first member of that structure is not a bitmask -- the three options
are mutually exclusive, and doing something like LOCK_SH | LOCK_UN is
semantically meaningless.

Consulting https://man7.org/linux/man-pages/man2/flock.2.html, nothing on
the flock() man page suggests that LOCK_SH, LOCK_EX and LOCK_UN can be used
as bitflags -- it so happens that they can in C, but this is not an API
guarantee. The kernel code for these flags handles things properly by first
removing the LOCK_NB flag and then doing equality comparisons against the
lock type -- not flag checks. Incidentally these get mapped to F_RDLCK,
F_WRLCK and F_UNLCK internally, which just so happen to have the same
values as LOCK_SH, LOCK_EX and LOCK_UN in PHP ;)

Is there some kind of evidence that people are actually trying to use these
as bitflags, and you're trying to solve a real problem here? Or is the only
problem being solved that somebody is celebrating their own ignorance and
incompetence over at r/lolphp again?

Regards,
Nikita