On Sun, Apr 24, 2022 at 12:14 PM Ilija Tovilo <tovilo.il...@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