Change the values of the LOCK_* constants

php.internals

Ilija Tovilo

4 years ago
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

Nikita Popov

4 years ago
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