Is this patch possible to get in php 5.x? What should I improve so my
patch would get reviewed?
I would need it in php 5.0 my self. It would come to comercial webserver
so there is required stable build which won't self build and patched
version. Specialy because this was first time I ever looked php source
code. (Happily using it about 5 years now)
Should I make some changes to documentation also? I didn't got time to
check it out yet how to change documentation.
I checked that 5.0 and 5.1 branches got same source for this file so
this patch could be applied for both of them. There was some minor
changes from 4.3 branch.
I tested this with 5.0 cvs branch (checkout today). It worked well after
I got sem_acquire parameter parsing converted to new functions. I got
little confused with that I must read everything to zval and then
convert it to what I want.
This patch is adds IPC_CREAT, IPC_EXCL and IPC_NOWAIT functionality to
sem_get and sem_acquire. The code changes add some constants and handles
those constants as parameters. There is possible issue that sem_get
could get unwanted parameters because values aren't checked before they
are passed to 'real' sem_get but that was functionality of old
implementation so I didn't want to change it.
Here is also changed sem_acquire to use new parameter parsing function.
I included 2 php test cases that I demo straits functionality of new
implemantion. test.php is not mean to output anything special.
test_nowait.php. Shows what happens if semaphore is tried to access same
time by 2 scripts. Execute test_nowait in 2 shells same time. The first
one to terminate should tell you that 2nd acquire was busy. 2nd should
tell that 2nd acquire was also successful.
http://cmax.gg/~coren/bug26610.patch
http://cmax.gg/~coren/test.phps
http://cmax.gg/~coren/test_nowait.phps
That's it. Thanks for comments and help how to improve this patch. Also
big thanks for reading to end. ;)
You got quite a few typos there. For example:
- fifeth_arg_force_ref (fifth)
- PHP_SEM_NOT_AVAILEBLE (AVAILABLE)
- allready
Following constants are against coding standards:
- "ENOENT",
- EEXIST
zval_dtor(error_code); should be zval_ptr_dtor(&error_code) to make sure
you protect reference counting.
In general, I suggest to go over the patch again and make sure you improve
things and then repost it to this list. Unfortunately I don't have time to
fix it myself.
Andi
At 09:52 AM 4/6/2005 +0300, Pauli wrote:
Is this patch possible to get in php 5.x? What should I improve so my
patch would get reviewed?I would need it in php 5.0 my self. It would come to comercial webserver
so there is required stable build which won't self build and patched
version. Specialy because this was first time I ever looked php source
code. (Happily using it about 5 years now)Should I make some changes to documentation also? I didn't got time to
check it out yet how to change documentation.I checked that 5.0 and 5.1 branches got same source for this file so
this patch could be applied for both of them. There was some minor
changes from 4.3 branch.I tested this with 5.0 cvs branch (checkout today). It worked well after
I got sem_acquire parameter parsing converted to new functions. I got
little confused with that I must read everything to zval and then convert
it to what I want.This patch is adds IPC_CREAT, IPC_EXCL and IPC_NOWAIT functionality to
sem_get and sem_acquire. The code changes add some constants and handles
those constants as parameters. There is possible issue that sem_get
could get unwanted parameters because values aren't checked before they
are passed to 'real' sem_get but that was functionality of old
implementation so I didn't want to change it.Here is also changed sem_acquire to use new parameter parsing function.
I included 2 php test cases that I demo straits functionality of new
implemantion. test.php is not mean to output anything special.
test_nowait.php. Shows what happens if semaphore is tried to access same
time by 2 scripts. Execute test_nowait in 2 shells same time. The first
one to terminate should tell you that 2nd acquire was busy. 2nd should
tell that 2nd acquire was also successful.http://cmax.gg/~coren/bug26610.patch
http://cmax.gg/~coren/test.phps
http://cmax.gg/~coren/test_nowait.phpsThat's it. Thanks for comments and help how to improve this patch. Also
big thanks for reading to end. ;)