Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:15885
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <5.1.0.14.2.20050406184709.02170b20@localhost>
Date: Wed, 06 Apr 2005 18:51:17 -0700
To: Pauli <horo@mbnet.fi>,internals@lists.php.net
In-Reply-To: <20050406065218.16267.qmail@lists.php.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Subject: Re: [PHP-DEV] [PATCH] Bug #26610 sem_get and sem_acquire
From: andi@zend.com (Andi Gutmans)
References: <20050406065218.16267.qmail@lists.php.net>

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.phps
>
>That's it. Thanks for comments and help how to improve this patch. Also
>big thanks for reading to end. ;)
>
>--
>PHP Internals - PHP Runtime Development Mailing List
>To unsubscribe, visit: http://www.php.net/unsub.php